From: Boris Ostrovsky <boris.ostrovsky@oracle.com> To: Zhenzhong Duan <zhenzhong.duan@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 Subject: Re: [PATCH v6 4/4] x86/xen: Add "nopv" support for HVM guest Date: Wed, 10 Jul 2019 09:21:08 -0400 [thread overview] Message-ID: <71b06655-79d2-97ae-cb6d-e0e606c8237f@oracle.com> (raw) In-Reply-To: <33876a98-9b6b-a1b9-e72f-352c3f95db89@oracle.com> On 7/9/19 10:07 PM, Zhenzhong Duan wrote: > 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 = { But hypervisors[] stays__initconst so that I thought could be a problem. But apparently it's not. > >> >> >>> + 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? Right. My point though is that this ordering is opaque to hypervisor-specific code and can change. The same is true about making assumptions about x86_platform/x86_init.hyper values. -boris
WARNING: multiple messages have this Message-ID (diff)
From: Boris Ostrovsky <boris.ostrovsky@oracle.com> To: Zhenzhong Duan <zhenzhong.duan@oracle.com>, linux-kernel@vger.kernel.org Cc: jgross@suse.com, sstabellini@kernel.org, mingo@redhat.com, bp@alien8.de, xen-devel@lists.xenproject.org, tglx@linutronix.de Subject: Re: [Xen-devel] [PATCH v6 4/4] x86/xen: Add "nopv" support for HVM guest Date: Wed, 10 Jul 2019 09:21:08 -0400 [thread overview] Message-ID: <71b06655-79d2-97ae-cb6d-e0e606c8237f@oracle.com> (raw) In-Reply-To: <33876a98-9b6b-a1b9-e72f-352c3f95db89@oracle.com> On 7/9/19 10:07 PM, Zhenzhong Duan wrote: > 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 = { But hypervisors[] stays__initconst so that I thought could be a problem. But apparently it's not. > >> >> >>> + 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? Right. My point though is that this ordering is opaque to hypervisor-specific code and can change. The same is true about making assumptions about x86_platform/x86_init.hyper values. -boris _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-07-10 13:20 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-07 9:15 [PATCH v6 0/4] misc fixes to PV extensions code Zhenzhong Duan 2019-07-07 9:15 ` [Xen-devel] " Zhenzhong Duan 2019-07-07 9:15 ` [PATCH v6 1/4] x86/xen: Mark xen_hvm_need_lapic() and xen_x2apic_para_available() as __init Zhenzhong Duan 2019-07-07 9:15 ` [Xen-devel] " Zhenzhong Duan 2019-07-07 9:15 ` [PATCH v6 2/4] x86: Add "nopv" parameter to disable PV extensions Zhenzhong Duan 2019-07-07 9:15 ` [Xen-devel] " Zhenzhong Duan 2019-07-07 9:15 ` [PATCH v6 3/4] xen: Map "xen_nopv" parameter to "nopv" and mark it obsolete Zhenzhong Duan 2019-07-07 9:15 ` [Xen-devel] " Zhenzhong Duan 2019-07-07 9:15 ` [PATCH v6 4/4] x86/xen: Add "nopv" support for HVM guest Zhenzhong Duan 2019-07-07 9:15 ` [Xen-devel] " Zhenzhong Duan 2019-07-08 13:46 ` Boris Ostrovsky 2019-07-08 13:46 ` [Xen-devel] " Boris Ostrovsky 2019-07-09 4:20 ` Zhenzhong Duan 2019-07-09 4:20 ` [Xen-devel] " Zhenzhong Duan 2019-07-09 14:54 ` Boris Ostrovsky 2019-07-09 14:54 ` [Xen-devel] " Boris Ostrovsky 2019-07-10 2:07 ` Zhenzhong Duan 2019-07-10 2:07 ` [Xen-devel] " Zhenzhong Duan 2019-07-10 13:21 ` Boris Ostrovsky [this message] 2019-07-10 13:21 ` Boris Ostrovsky
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=71b06655-79d2-97ae-cb6d-e0e606c8237f@oracle.com \ --to=boris.ostrovsky@oracle.com \ --cc=bp@alien8.de \ --cc=jgross@suse.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mingo@redhat.com \ --cc=sstabellini@kernel.org \ --cc=tglx@linutronix.de \ --cc=xen-devel@lists.xenproject.org \ --cc=zhenzhong.duan@oracle.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.