Re: [Xen-devel] [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and IOAPIC
On 10/26/2016 11:18 AM, Roger Pau Monné wrote: > On Wed, Oct 26, 2016 at 10:50:21AM -0400, Boris Ostrovsky wrote: >> On 10/26/2016 06:42 AM, Roger Pau Monné wrote: >>> On Fri, Oct 14, 2016 at 02:05:15PM -0400, Boris Ostrovsky wrote: Make sure they don't use these devices since they are not emulated for unprivileged PVH guest. Also don't initialize hypercall page for them in init_hvm_pv_info() since this has already been done. Signed-off-by: Boris Ostrovsky --- arch/x86/xen/enlighten.c | 24 +--- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index d38d568..6c1a330 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -1803,10 +1803,21 @@ static void __init init_hvm_pv_info(void) minor = eax & 0x; printk(KERN_INFO "Xen version %d.%d.\n", major, minor); - cpuid(base + 2, &pages, &msr, &ecx, &edx); + xen_domain_type = XEN_HVM_DOMAIN; - pfn = __pa(hypercall_page); - wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); + /* PVH set up hypercall page earlier in xen_prepare_pvh() */ + if (xen_pvh_domain()) { + pv_info.name = "Xen PVH"; +#ifdef CONFIG_ACPI + /* No PIC or IOAPIC */ >>> Shouldn't this be fetched from the MADT ACPI table if ACPI is available >>> (rsdp_paddr != 0 in start_info)? >> >> At this point we haven't parsed ACPI yet (with or without rsdp_paddr, >> which we don't set anyway for domU) so we don't know whether we have PIC >> or IOAPIC. > I guess I'm missing something, but if we are providing ACPI tables to a PVH > DomU rsdp_paddr should be set, or else we are failing to comply with our own > start info specification. acpi_find_root_pointer() searches low MB of memory for the RSDP signature. This is standard ACPICA's method of finding it, I'd think that FreeBSD does the same thing. And that's how a non-UEFI system is expected to find it as required by the ACPI spec ("Root System Description Pointer (RSDP)" section) RSDP structure is placed at RSDP_ADDRESS (which is just under 1MB at 0xfffc0) by libxl__dom_load_acpi(). > >> Having said that, I will probably remove this ("acpi_irq_model = >> ACPI_IRQ_MODEL_PLATFORM", together with the comment) since I am working >> on adding SCI support via an event channel and ACPI_IRQ_MODEL_PIC model, >> which is default, seems to work OK. > Hm, right, that might be an option. I'd actually prefer not to use ACPI_IRQ_MODEL_PIC since it implies that we have a PIC (which we don't). It just so happens that using it in Linux works for PVH as well. > Do you know how hardware-reduced ACPI > implementations (which IIRC also don't have a SCI interrupt) deliver events > to the OS? Or it's simply not possible in that case? I believe you need _AEI object for that. At least that's what section "4.1.1.1 GPIO-Signaled Events" suggests. -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and IOAPIC
On Wed, Oct 26, 2016 at 10:50:21AM -0400, Boris Ostrovsky wrote: > On 10/26/2016 06:42 AM, Roger Pau Monné wrote: > > On Fri, Oct 14, 2016 at 02:05:15PM -0400, Boris Ostrovsky wrote: > >> Make sure they don't use these devices since they are not emulated > >> for unprivileged PVH guest. > >> > >> Also don't initialize hypercall page for them in init_hvm_pv_info() > >> since this has already been done. > >> > >> Signed-off-by: Boris Ostrovsky > >> --- > >> arch/x86/xen/enlighten.c | 24 +--- > >> 1 file changed, 17 insertions(+), 7 deletions(-) > >> > >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > >> index d38d568..6c1a330 100644 > >> --- a/arch/x86/xen/enlighten.c > >> +++ b/arch/x86/xen/enlighten.c > >> @@ -1803,10 +1803,21 @@ static void __init init_hvm_pv_info(void) > >>minor = eax & 0x; > >>printk(KERN_INFO "Xen version %d.%d.\n", major, minor); > >> > >> - cpuid(base + 2, &pages, &msr, &ecx, &edx); > >> + xen_domain_type = XEN_HVM_DOMAIN; > >> > >> - pfn = __pa(hypercall_page); > >> - wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); > >> + /* PVH set up hypercall page earlier in xen_prepare_pvh() */ > >> + if (xen_pvh_domain()) { > >> + pv_info.name = "Xen PVH"; > >> +#ifdef CONFIG_ACPI > >> + /* No PIC or IOAPIC */ > > Shouldn't this be fetched from the MADT ACPI table if ACPI is available > > (rsdp_paddr != 0 in start_info)? > > > At this point we haven't parsed ACPI yet (with or without rsdp_paddr, > which we don't set anyway for domU) so we don't know whether we have PIC > or IOAPIC. I guess I'm missing something, but if we are providing ACPI tables to a PVH DomU rsdp_paddr should be set, or else we are failing to comply with our own start info specification. > Having said that, I will probably remove this ("acpi_irq_model = > ACPI_IRQ_MODEL_PLATFORM", together with the comment) since I am working > on adding SCI support via an event channel and ACPI_IRQ_MODEL_PIC model, > which is default, seems to work OK. Hm, right, that might be an option. Do you know how hardware-reduced ACPI implementations (which IIRC also don't have a SCI interrupt) deliver events to the OS? Or it's simply not possible in that case? Roger. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and IOAPIC
On 10/26/2016 06:42 AM, Roger Pau Monné wrote: > On Fri, Oct 14, 2016 at 02:05:15PM -0400, Boris Ostrovsky wrote: >> Make sure they don't use these devices since they are not emulated >> for unprivileged PVH guest. >> >> Also don't initialize hypercall page for them in init_hvm_pv_info() >> since this has already been done. >> >> Signed-off-by: Boris Ostrovsky >> --- >> arch/x86/xen/enlighten.c | 24 +--- >> 1 file changed, 17 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >> index d38d568..6c1a330 100644 >> --- a/arch/x86/xen/enlighten.c >> +++ b/arch/x86/xen/enlighten.c >> @@ -1803,10 +1803,21 @@ static void __init init_hvm_pv_info(void) >> minor = eax & 0x; >> printk(KERN_INFO "Xen version %d.%d.\n", major, minor); >> >> -cpuid(base + 2, &pages, &msr, &ecx, &edx); >> +xen_domain_type = XEN_HVM_DOMAIN; >> >> -pfn = __pa(hypercall_page); >> -wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); >> +/* PVH set up hypercall page earlier in xen_prepare_pvh() */ >> +if (xen_pvh_domain()) { >> +pv_info.name = "Xen PVH"; >> +#ifdef CONFIG_ACPI >> +/* No PIC or IOAPIC */ > Shouldn't this be fetched from the MADT ACPI table if ACPI is available > (rsdp_paddr != 0 in start_info)? At this point we haven't parsed ACPI yet (with or without rsdp_paddr, which we don't set anyway for domU) so we don't know whether we have PIC or IOAPIC. Having said that, I will probably remove this ("acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM", together with the comment) since I am working on adding SCI support via an event channel and ACPI_IRQ_MODEL_PIC model, which is default, seems to work OK. Alternatively, I may keep ACPI_IRQ_MODEL_PLATFORM but then I'd need to make changes to acpi_gsi_to_irq(). -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and IOAPIC
On Fri, Oct 14, 2016 at 02:05:15PM -0400, Boris Ostrovsky wrote: > Make sure they don't use these devices since they are not emulated > for unprivileged PVH guest. > > Also don't initialize hypercall page for them in init_hvm_pv_info() > since this has already been done. > > Signed-off-by: Boris Ostrovsky > --- > arch/x86/xen/enlighten.c | 24 +--- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index d38d568..6c1a330 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -1803,10 +1803,21 @@ static void __init init_hvm_pv_info(void) > minor = eax & 0x; > printk(KERN_INFO "Xen version %d.%d.\n", major, minor); > > - cpuid(base + 2, &pages, &msr, &ecx, &edx); > + xen_domain_type = XEN_HVM_DOMAIN; > > - pfn = __pa(hypercall_page); > - wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); > + /* PVH set up hypercall page earlier in xen_prepare_pvh() */ > + if (xen_pvh_domain()) { > + pv_info.name = "Xen PVH"; > +#ifdef CONFIG_ACPI > + /* No PIC or IOAPIC */ Shouldn't this be fetched from the MADT ACPI table if ACPI is available (rsdp_paddr != 0 in start_info)? Roger. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and IOAPIC
On 10/14/2016 03:16 PM, Konrad Rzeszutek Wilk wrote: > On Fri, Oct 14, 2016 at 02:05:15PM -0400, Boris Ostrovsky wrote: >> Make sure they don't use these devices since they are not emulated >> for unprivileged PVH guest. > Which means they would just return 0 ? Or would it get worst since > the in/out would go to the hypervisor which would kill the guest? For PIC and IOAPIC (and I think RTC too) we will get a warning (with a splat) that SCI cannot be initialized. But the guest will continue running without problems. -boris >> Also don't initialize hypercall page for them in init_hvm_pv_info() >> since this has already been done. >> >> Signed-off-by: Boris Ostrovsky >> --- >> arch/x86/xen/enlighten.c | 24 +--- >> 1 file changed, 17 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c >> index d38d568..6c1a330 100644 >> --- a/arch/x86/xen/enlighten.c >> +++ b/arch/x86/xen/enlighten.c >> @@ -1803,10 +1803,21 @@ static void __init init_hvm_pv_info(void) >> minor = eax & 0x; >> printk(KERN_INFO "Xen version %d.%d.\n", major, minor); >> >> -cpuid(base + 2, &pages, &msr, &ecx, &edx); >> +xen_domain_type = XEN_HVM_DOMAIN; >> >> -pfn = __pa(hypercall_page); >> -wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); >> +/* PVH set up hypercall page earlier in xen_prepare_pvh() */ > A period at the end? >> +if (xen_pvh_domain()) { >> +pv_info.name = "Xen PVH"; >> +#ifdef CONFIG_ACPI >> +/* No PIC or IOAPIC */ > Here? >> +acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM; >> +#endif >> +} else { >> +pv_info.name = "Xen HVM"; >> +cpuid(base + 2, &pages, &msr, &ecx, &edx); > Could you use cpuid_ebx ? >> +pfn = __pa(hypercall_page); >> +wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); >> +} >> >> xen_setup_features(); >> >> @@ -1815,10 +1826,6 @@ static void __init init_hvm_pv_info(void) >> this_cpu_write(xen_vcpu_id, ebx); >> else >> this_cpu_write(xen_vcpu_id, smp_processor_id()); >> - >> -pv_info.name = "Xen HVM"; >> - >> -xen_domain_type = XEN_HVM_DOMAIN; >> } >> >> static int xen_cpu_up_prepare(unsigned int cpu) >> @@ -1892,6 +1899,9 @@ static void __init xen_hvm_guest_init(void) >> >> init_hvm_pv_info(); >> >> +if (xen_pvh_domain()) >> +x86_platform.legacy.rtc = 0; >> + >> xen_hvm_init_shared_info(); >> >> xen_panic_handler_init(); >> -- >> 1.8.3.1 >> >> >> ___ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> https://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and IOAPIC
On Fri, Oct 14, 2016 at 02:05:15PM -0400, Boris Ostrovsky wrote: > Make sure they don't use these devices since they are not emulated > for unprivileged PVH guest. Which means they would just return 0 ? Or would it get worst since the in/out would go to the hypervisor which would kill the guest? > > Also don't initialize hypercall page for them in init_hvm_pv_info() > since this has already been done. > > Signed-off-by: Boris Ostrovsky > --- > arch/x86/xen/enlighten.c | 24 +--- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index d38d568..6c1a330 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -1803,10 +1803,21 @@ static void __init init_hvm_pv_info(void) > minor = eax & 0x; > printk(KERN_INFO "Xen version %d.%d.\n", major, minor); > > - cpuid(base + 2, &pages, &msr, &ecx, &edx); > + xen_domain_type = XEN_HVM_DOMAIN; > > - pfn = __pa(hypercall_page); > - wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); > + /* PVH set up hypercall page earlier in xen_prepare_pvh() */ A period at the end? > + if (xen_pvh_domain()) { > + pv_info.name = "Xen PVH"; > +#ifdef CONFIG_ACPI > + /* No PIC or IOAPIC */ Here? > + acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM; > +#endif > + } else { > + pv_info.name = "Xen HVM"; > + cpuid(base + 2, &pages, &msr, &ecx, &edx); Could you use cpuid_ebx ? > + pfn = __pa(hypercall_page); > + wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); > + } > > xen_setup_features(); > > @@ -1815,10 +1826,6 @@ static void __init init_hvm_pv_info(void) > this_cpu_write(xen_vcpu_id, ebx); > else > this_cpu_write(xen_vcpu_id, smp_processor_id()); > - > - pv_info.name = "Xen HVM"; > - > - xen_domain_type = XEN_HVM_DOMAIN; > } > > static int xen_cpu_up_prepare(unsigned int cpu) > @@ -1892,6 +1899,9 @@ static void __init xen_hvm_guest_init(void) > > init_hvm_pv_info(); > > + if (xen_pvh_domain()) > + x86_platform.legacy.rtc = 0; > + > xen_hvm_init_shared_info(); > > xen_panic_handler_init(); > -- > 1.8.3.1 > > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and IOAPIC
Make sure they don't use these devices since they are not emulated for unprivileged PVH guest. Also don't initialize hypercall page for them in init_hvm_pv_info() since this has already been done. Signed-off-by: Boris Ostrovsky --- arch/x86/xen/enlighten.c | 24 +--- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index d38d568..6c1a330 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -1803,10 +1803,21 @@ static void __init init_hvm_pv_info(void) minor = eax & 0x; printk(KERN_INFO "Xen version %d.%d.\n", major, minor); - cpuid(base + 2, &pages, &msr, &ecx, &edx); + xen_domain_type = XEN_HVM_DOMAIN; - pfn = __pa(hypercall_page); - wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); + /* PVH set up hypercall page earlier in xen_prepare_pvh() */ + if (xen_pvh_domain()) { + pv_info.name = "Xen PVH"; +#ifdef CONFIG_ACPI + /* No PIC or IOAPIC */ + acpi_irq_model = ACPI_IRQ_MODEL_PLATFORM; +#endif + } else { + pv_info.name = "Xen HVM"; + cpuid(base + 2, &pages, &msr, &ecx, &edx); + pfn = __pa(hypercall_page); + wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32)); + } xen_setup_features(); @@ -1815,10 +1826,6 @@ static void __init init_hvm_pv_info(void) this_cpu_write(xen_vcpu_id, ebx); else this_cpu_write(xen_vcpu_id, smp_processor_id()); - - pv_info.name = "Xen HVM"; - - xen_domain_type = XEN_HVM_DOMAIN; } static int xen_cpu_up_prepare(unsigned int cpu) @@ -1892,6 +1899,9 @@ static void __init xen_hvm_guest_init(void) init_hvm_pv_info(); + if (xen_pvh_domain()) + x86_platform.legacy.rtc = 0; + xen_hvm_init_shared_info(); xen_panic_handler_init(); -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel