Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v6 4/4] x86/xen: Add "nopv" support for HVM guest
To:     Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        linux-kernel@vger.kernel.org
Cc:     xen-devel@lists.xenproject.org, jgross@suse.com,
        sstabellini@kernel.org, tglx@linutronix.de, mingo@redhat.com,
        bp@alien8.de
References: <1562490908-17882-1-git-send-email-zhenzhong.duan@oracle.com>
 <1562490908-17882-5-git-send-email-zhenzhong.duan@oracle.com>
 <86b0dbb9-74a7-6883-e6d7-210bd35a6944@oracle.com>
 <6cbd7b78-3d8d-64ae-fd2e-82244dbe6a1e@oracle.com>
 <3d7db7c6-cea3-9dce-0519-a1c600b33273@oracle.com>
From:   Zhenzhong Duan <zhenzhong.duan@oracle.com>
Organization: Oracle Corporation
Message-ID: <33876a98-9b6b-a1b9-e72f-352c3f95db89@oracle.com>
Date:   Wed, 10 Jul 2019 10:07:54 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <3d7db7c6-cea3-9dce-0519-a1c600b33273@oracle.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2019/7/9 22:54, Boris Ostrovsky wrote:
> On 7/9/19 12:20 AM, Zhenzhong Duan wrote:
>> -const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
>> +static uint32_t __init xen_platform_hvm(void)
>> +{
>> +       uint32_t xen_domain = xen_cpuid_base();
>> +       struct x86_hyper_init *h = &x86_hyper_xen_hvm.init;
>> +
>> +       if (xen_pv_domain())
>> +               return 0;
>> +
>> +       if (xen_pvh_domain() && nopv) {
>> +               /* Guest booting via the Xen-PVH boot entry goes here */
>> +               pr_info("\"nopv\" parameter is ignored in PVH guest\n");
>> +               nopv = false;
>> +       } else if (nopv && xen_domain) {
>> +               /*
>> +                * Guest booting via normal boot entry (like via
>> grub2) goes
>> +                * here.
>> +                *
>> +                * Use interface functions for bare hardware if nopv,
>> +                * xen_hvm_guest_late_init is an exception as we need to
>> +                * detect PVH and panic there.
>> +                */
>> +               memcpy(h, (void *)&x86_init.hyper,
>> sizeof(x86_init.hyper));
>
> And this worked? I'd think it would fail since h points to RO section.
Yes, I have below changes in the patch.

-const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
+struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {

>
>
>> +               memcpy(&x86_hyper_xen_hvm.runtime, (void
>> *)&x86_platform.hyper,
>> +                      sizeof(x86_platform.hyper));
>> +               h->guest_late_init = xen_hvm_guest_late_init;
>
> To me this still doesn't look right --- you are making assumptions about
> x86_platform/x86_init.hyper and I don't think you can assume they have
> not been set to point to another hypervisor, for example.

You mean copy_array() calls in init_hypervisor_platform()? But that 
happens after

detect_hypervisor_vendor() shoose out the prefered hypervisor. In detect 
stage,

x86_platform/x86_init.hyper has default value for bare hardware, or I 
missed something?

Just realized I can use memset to zero instead of memcpy which looks 
more rational.

>
> Would modifying all x86_hyper_xen_hvm's ops (except, I guess,
> xen_hvm_guest_late_init()) to immediately return if nopv is set work?

I think so,  Let me try it.

Zhenzhong