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