Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1501116 > unrolled thread

[PATCH 0/8] PVH v2 support

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/8] PVH v2 support Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 20:20 +0200
    [PATCH 7/8] xen/pvh: PVH guests always have PV devices Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 20:20 +0200
      Re: [Xen-devel] [PATCH 7/8] xen/pvh: PVH guests always have PV  devices Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2016-10-14 21:30 +0200
      Re: [PATCH 7/8] xen/pvh: PVH guests always have PV devices Juergen Gross <jgross@suse.com> - 2016-10-18 18:00 +0200
    [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and IOAPIC Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 20:20 +0200
      Re: [Xen-devel] [PATCH 5/8] xen/pvh: Prevent PVH guests from using  PIC, RTC and IOAPIC Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2016-10-14 21:20 +0200
        Re: [Xen-devel] [PATCH 5/8] xen/pvh: Prevent PVH guests from using  PIC, RTC and IOAPIC Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 21:40 +0200
      Re: [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and  IOAPIC Roger Pau Monné <roger.pau@citrix.com> - 2016-10-26 12:50 +0200
        Re: [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and  IOAPIC Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-26 16:50 +0200
          Re: [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and  IOAPIC Roger Pau Monné <roger.pau@citrix.com> - 2016-10-26 17:20 +0200
            Re: [PATCH 5/8] xen/pvh: Prevent PVH guests from using PIC, RTC and  IOAPIC Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-26 18:10 +0200
    [PATCH 8/8] xen/pvh: Enable CPU hotplug Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 20:20 +0200
      Re: [Xen-devel] [PATCH 8/8] xen/pvh: Enable CPU hotplug Andrew Cooper <andrew.cooper3@citrix.com> - 2016-10-14 20:50 +0200
        Re: [Xen-devel] [PATCH 8/8] xen/pvh: Enable CPU hotplug Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 21:10 +0200
    [PATCH 4/8] xen/pvh: Bootstrap PVH guest Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 20:20 +0200
      Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest Andrew Cooper <andrew.cooper3@citrix.com> - 2016-10-14 20:50 +0200
        Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 21:00 +0200
          Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest Andrew Cooper <andrew.cooper3@citrix.com> - 2016-10-14 21:20 +0200
      Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2016-10-14 21:20 +0200
        Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 21:40 +0200
    [PATCH 2/8] x86/head: Refactor 32-bit pgtable setup Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 20:20 +0200
      Re: [PATCH 2/8] x86/head: Refactor 32-bit pgtable setup hpa@zytor.com - 2016-10-14 20:40 +0200
        Re: [PATCH 2/8] x86/head: Refactor 32-bit pgtable setup Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 20:50 +0200
          Re: [PATCH 2/8] x86/head: Refactor 32-bit pgtable setup hpa@zytor.com - 2016-10-14 21:10 +0200
            Re: [PATCH 2/8] x86/head: Refactor 32-bit pgtable setup Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 21:30 +0200
    [PATCH 3/8] xen/pvh: Import PVH-related Xen public interfaces Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 20:20 +0200
      Re: [Xen-devel] [PATCH 3/8] xen/pvh: Import PVH-related Xen public  interfaces Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2016-10-14 20:40 +0200
      Re: [PATCH 3/8] xen/pvh: Import PVH-related Xen public interfaces Juergen Gross <jgross@suse.com> - 2016-10-21 13:00 +0200
    [PATCH 1/8] xen/x86: Remove PVH support Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-14 20:20 +0200
      Re: [Xen-devel] [PATCH 1/8] xen/x86: Remove PVH support Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> - 2016-10-14 20:40 +0200
      Re: [PATCH 1/8] xen/x86: Remove PVH support Juergen Gross <jgross@suse.com> - 2016-10-18 15:50 +0200
        Re: [PATCH 1/8] xen/x86: Remove PVH support Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-18 16:50 +0200
          Re: [PATCH 1/8] xen/x86: Remove PVH support Juergen Gross <jgross@suse.com> - 2016-10-18 17:40 +0200
            Re: [PATCH 1/8] xen/x86: Remove PVH support Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-10-18 17:50 +0200

Page 1 of 2  [1] 2  Next page →

#1501116 — [PATCH 0/8] PVH v2 support

(Resending with proper mailing lists included)

PVH v2 support for unprivileged guests.

Previous version was posted long time ago. Major changes:
1. Drop PVH v1 support
2. Enable ACPI. This allows us to use much more of native code and
   results in making this series much simpler (for example, PV-style
   VCPU bringup is no longer necessary).
3. Refactor 32-bit pagetable setup from native code (by Matt Fleming)

This has been tested on Intel/AMD, both 32- and 64-bit, including CPU
hotplug and save/restore. Compile-tested for ARM.


Boris Ostrovsky (8):
  xen/x86: Remove PVH support
  x86/head: Refactor 32-bit pgtable setup
  xen/pvh: Import PVH-related Xen public interfaces
  xen/pvh: Bootstrap PVH guest
  xen/pvh: Prevent PVH guests from using PIC, RTC and IOAPIC
  xen/pvh: Initialize grant table for PVH guests
  xen/pvh: PVH guests always have PV devices
  xen/pvh: Enable CPU hotplug

 arch/x86/Makefile                      |   2 +
 arch/x86/kernel/Makefile               |   2 +
 arch/x86/kernel/head_32.S              | 168 +---------------------
 arch/x86/kernel/pgtable_32.S           | 196 +++++++++++++++++++++++++
 arch/x86/xen/Kconfig                   |   2 +-
 arch/x86/xen/Makefile                  |   1 +
 arch/x86/xen/enlighten.c               | 251 ++++++++++++++++-----------------
 arch/x86/xen/mmu.c                     |  21 +--
 arch/x86/xen/platform-pci-unplug.c     |   4 +-
 arch/x86/xen/setup.c                   |  37 +----
 arch/x86/xen/smp.c                     |  78 ++++------
 arch/x86/xen/smp.h                     |   8 --
 arch/x86/xen/xen-head.S                |  62 +-------
 arch/x86/xen/xen-ops.h                 |   1 -
 arch/x86/xen/xen-pvh.S                 | 143 +++++++++++++++++++
 drivers/xen/cpu_hotplug.c              |   2 +-
 drivers/xen/events/events_base.c       |   1 -
 drivers/xen/grant-table.c              |   8 +-
 include/xen/interface/elfnote.h        |  12 +-
 include/xen/interface/hvm/hvm_vcpu.h   | 143 +++++++++++++++++++
 include/xen/interface/hvm/start_info.h |  98 +++++++++++++
 include/xen/xen.h                      |  12 +-
 22 files changed, 764 insertions(+), 488 deletions(-)
 create mode 100644 arch/x86/kernel/pgtable_32.S
 create mode 100644 arch/x86/xen/xen-pvh.S
 create mode 100644 include/xen/interface/hvm/hvm_vcpu.h
 create mode 100644 include/xen/interface/hvm/start_info.h

-- 
1.8.3.1

[toc] | [next] | [standalone]

#1501117 — [PATCH 7/8] xen/pvh: PVH guests always have PV devices

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/platform-pci-unplug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
index 90d1b83..33a783c 100644
--- a/arch/x86/xen/platform-pci-unplug.c
+++ b/arch/x86/xen/platform-pci-unplug.c
@@ -73,8 +73,8 @@ bool xen_has_pv_devices(void)
 	if (!xen_domain())
 		return false;
 
-	/* PV domains always have them. */
-	if (xen_pv_domain())
+	/* PV and PVH domains always have them. */
+	if (xen_pv_domain() || xen_pvh_domain())
 		return true;
 
 	/* And user has xen_platform_pci=0 set in guest config as
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1501165 — Re: [Xen-devel] [PATCH 7/8] xen/pvh: PVH guests always have PV devices

On Fri, Oct 14, 2016 at 02:05:17PM -0400, Boris Ostrovsky wrote:
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/xen/platform-pci-unplug.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
> index 90d1b83..33a783c 100644
> --- a/arch/x86/xen/platform-pci-unplug.c
> +++ b/arch/x86/xen/platform-pci-unplug.c
> @@ -73,8 +73,8 @@ bool xen_has_pv_devices(void)
>  	if (!xen_domain())
>  		return false;
>  
> -	/* PV domains always have them. */
> -	if (xen_pv_domain())
> +	/* PV and PVH domains always have them. */
> +	if (xen_pv_domain() || xen_pvh_domain())
>  		return true;
>  
>  	/* And user has xen_platform_pci=0 set in guest config as
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

[toc] | [prev] | [next] | [standalone]

#1503153 — Re: [PATCH 7/8] xen/pvh: PVH guests always have PV devices

On 14/10/16 20:05, Boris Ostrovsky wrote:
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Reviewed-by: Juergen Gross <jgross@suse.com>

> ---
>  arch/x86/xen/platform-pci-unplug.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
> index 90d1b83..33a783c 100644
> --- a/arch/x86/xen/platform-pci-unplug.c
> +++ b/arch/x86/xen/platform-pci-unplug.c
> @@ -73,8 +73,8 @@ bool xen_has_pv_devices(void)
>  	if (!xen_domain())
>  		return false;
>  
> -	/* PV domains always have them. */
> -	if (xen_pv_domain())
> +	/* PV and PVH domains always have them. */
> +	if (xen_pv_domain() || xen_pvh_domain())
>  		return true;
>  
>  	/* And user has xen_platform_pci=0 set in guest config as
> 

[toc] | [prev] | [next] | [standalone]

#1501118 — [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 <boris.ostrovsky@oracle.com>
---
 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 & 0xffff;
 	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

[toc] | [prev] | [next] | [standalone]

#1501156 — 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 <boris.ostrovsky@oracle.com>
> ---
>  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 & 0xffff;
>  	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

[toc] | [prev] | [next] | [standalone]

#1501167 — 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 <boris.ostrovsky@oracle.com>
>> ---
>>  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 & 0xffff;
>>  	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

[toc] | [prev] | [next] | [standalone]

#1509045 — Re: [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 <boris.ostrovsky@oracle.com>
> ---
>  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 & 0xffff;
>  	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.

[toc] | [prev] | [next] | [standalone]

#1509490 — Re: [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 <boris.ostrovsky@oracle.com>
>> ---
>>  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 & 0xffff;
>>  	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

[toc] | [prev] | [next] | [standalone]

#1509522 — Re: [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 <boris.ostrovsky@oracle.com>
> >> ---
> >>  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 & 0xffff;
> >>  	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.

[toc] | [prev] | [next] | [standalone]

#1509566 — Re: [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 <boris.ostrovsky@oracle.com>
>>>> ---
>>>>  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 & 0xffff;
>>>>  	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

[toc] | [prev] | [next] | [standalone]

#1501119 — [PATCH 8/8] xen/pvh: Enable CPU hotplug

PVH guests don't receive ACPI hotplug interrupts and therefore
need to monitor xenstore for CPU hotplug event.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 drivers/xen/cpu_hotplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index 5676aef..0bab60a3 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -107,7 +107,7 @@ static int __init setup_vcpu_hotplug_event(void)
 		.notifier_call = setup_cpu_watcher };
 
 #ifdef CONFIG_X86
-	if (!xen_pv_domain())
+	if (!xen_pv_domain() && !xen_pvh_domain())
 #else
 	if (!xen_domain())
 #endif
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1501144 — Re: [Xen-devel] [PATCH 8/8] xen/pvh: Enable CPU hotplug

On 14/10/16 19:05, Boris Ostrovsky wrote:
> PVH guests don't receive ACPI hotplug interrupts and therefore
> need to monitor xenstore for CPU hotplug event.

Why not?  If they don't, they should.  As we are providing ACPI anyway,
we should provide all bits of it.

~Andrew

[toc] | [prev] | [next] | [standalone]

#1501153 — Re: [Xen-devel] [PATCH 8/8] xen/pvh: Enable CPU hotplug

On 10/14/2016 02:41 PM, Andrew Cooper wrote:
> On 14/10/16 19:05, Boris Ostrovsky wrote:
>> PVH guests don't receive ACPI hotplug interrupts and therefore
>> need to monitor xenstore for CPU hotplug event.
> Why not?  If they don't, they should.  As we are providing ACPI anyway,
> we should provide all bits of it.

We don't have IOAPIC, which is how these interrupts are typically
delivered. I suppose we might be able to specify it as something else.

I'll look into this.

-boris

[toc] | [prev] | [next] | [standalone]

#1501120 — [PATCH 4/8] xen/pvh: Bootstrap PVH guest

Start PVH guest at XEN_ELFNOTE_PHYS32_ENTRY address. Setup hypercall
page, initialize boot_params, enable early page tables.

Since this stub is executed before kernel entry point we cannot use
variables in .bss which is cleared by kernel. We explicitly place
variables that are initialized here into .data.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/xen/Kconfig     |   2 +-
 arch/x86/xen/Makefile    |   1 +
 arch/x86/xen/enlighten.c |  87 +++++++++++++++++++++++++++-
 arch/x86/xen/xen-pvh.S   | 143 +++++++++++++++++++++++++++++++++++++++++++++++
 include/xen/xen.h        |   5 ++
 5 files changed, 236 insertions(+), 2 deletions(-)
 create mode 100644 arch/x86/xen/xen-pvh.S

diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
index c7b15f3..76b6dbd 100644
--- a/arch/x86/xen/Kconfig
+++ b/arch/x86/xen/Kconfig
@@ -53,5 +53,5 @@ config XEN_DEBUG_FS
 
 config XEN_PVH
 	bool "Support for running as a PVH guest"
-	depends on X86_64 && XEN && XEN_PVHVM
+	depends on XEN && XEN_PVHVM && ACPI
 	def_bool n
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index e47e527..cb0164a 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_XEN_DEBUG_FS)	+= debugfs.o
 obj-$(CONFIG_XEN_DOM0)		+= vga.o
 obj-$(CONFIG_SWIOTLB_XEN)	+= pci-swiotlb-xen.o
 obj-$(CONFIG_XEN_EFI)		+= efi.o
+obj-$(CONFIG_XEN_PVH)	 	+= xen-pvh.o
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index dc4ed0c..d38d568 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -45,6 +45,7 @@
 #include <xen/interface/memory.h>
 #include <xen/interface/nmi.h>
 #include <xen/interface/xen-mca.h>
+#include <xen/interface/hvm/start_info.h>
 #include <xen/features.h>
 #include <xen/page.h>
 #include <xen/hvm.h>
@@ -121,7 +122,8 @@
 DEFINE_PER_CPU(uint32_t, xen_vcpu_id);
 EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
 
-enum xen_domain_type xen_domain_type = XEN_NATIVE;
+enum xen_domain_type xen_domain_type
+	__attribute__((section(".data"))) = XEN_NATIVE;
 EXPORT_SYMBOL_GPL(xen_domain_type);
 
 unsigned long *machine_to_phys_mapping = (void *)MACH2PHYS_VIRT_START;
@@ -176,6 +178,17 @@ struct tls_descs {
  */
 static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
 
+#ifdef CONFIG_XEN_PVH
+/*
+ * PVH variables. These need to live in data segment since they are
+ * initialized before startup_{32|64}, which clear .bss, are invoked.
+ */
+int xen_pvh __attribute__((section(".data"))) = 0;
+struct hvm_start_info pvh_start_info __attribute__((section(".data")));
+uint pvh_start_info_sz = sizeof(pvh_start_info);
+struct boot_params pvh_bootparams __attribute__((section(".data")));
+#endif
+
 static void clamp_max_cpus(void)
 {
 #ifdef CONFIG_SMP
@@ -1669,6 +1682,78 @@ asmlinkage __visible void __init xen_start_kernel(void)
 #endif
 }
 
+#ifdef CONFIG_XEN_PVH
+static void __init init_pvh_bootparams(void)
+{
+	struct xen_memory_map memmap;
+	int i;
+
+	memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
+
+	memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_map);
+	set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_map);
+	if (HYPERVISOR_memory_op(XENMEM_memory_map, &memmap)) {
+		xen_raw_console_write("XENMEM_memory_map failed\n");
+		BUG();
+	}
+
+	pvh_bootparams.e820_map[memmap.nr_entries].addr =
+		ISA_START_ADDRESS;
+	pvh_bootparams.e820_map[memmap.nr_entries].size =
+		ISA_END_ADDRESS - ISA_START_ADDRESS;
+	pvh_bootparams.e820_map[memmap.nr_entries++].type =
+		E820_RESERVED;
+
+	sanitize_e820_map(pvh_bootparams.e820_map,
+			  ARRAY_SIZE(pvh_bootparams.e820_map),
+			  &memmap.nr_entries);
+
+	pvh_bootparams.e820_entries = memmap.nr_entries;
+	for (i = 0; i < pvh_bootparams.e820_entries; i++)
+		e820_add_region(pvh_bootparams.e820_map[i].addr,
+				pvh_bootparams.e820_map[i].size,
+				pvh_bootparams.e820_map[i].type);
+
+	pvh_bootparams.hdr.cmd_line_ptr =
+		pvh_start_info.cmdline_paddr;
+
+	/* The first module is always ramdisk */
+	if (pvh_start_info.nr_modules) {
+		struct hvm_modlist_entry *modaddr =
+			__va(pvh_start_info.modlist_paddr);
+		pvh_bootparams.hdr.ramdisk_image = modaddr->paddr;
+		pvh_bootparams.hdr.ramdisk_size = modaddr->size;
+	}
+
+	/*
+	 * See Documentation/x86/boot.txt.
+	 *
+	 * Version 2.12 supports Xen entry point but we will use default x86/PC
+	 * environment (i.e. hardware_subarch 0).
+	 */
+	pvh_bootparams.hdr.version = 0x212;
+	pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */
+}
+
+/*
+ * This routine (and those that it might call) should not use
+ * anything that lives in .bss since that segment will be cleared later
+ */
+void __init xen_prepare_pvh(void)
+{
+	u32 eax, ecx, edx, msr;
+	u64 pfn;
+
+	xen_pvh = 1;
+
+	cpuid(xen_cpuid_base() + 2, &eax, &msr, &ecx, &edx);
+	pfn = __pa(hypercall_page);
+	wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32));
+
+	init_pvh_bootparams();
+}
+#endif
+
 void __ref xen_hvm_init_shared_info(void)
 {
 	int cpu;
diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
new file mode 100644
index 0000000..58c477b
--- /dev/null
+++ b/arch/x86/xen/xen-pvh.S
@@ -0,0 +1,143 @@
+/*
+ * Copyright C 2016, Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+	.code32
+	.text
+#define _pa(x)          ((x) - __START_KERNEL_map)
+
+#include <linux/elfnote.h>
+#include <linux/init.h>
+#include <linux/linkage.h>
+#include <asm/segment.h>
+#include <asm/asm.h>
+#include <asm/boot.h>
+#include <asm/processor-flags.h>
+#include <asm/msr.h>
+#include <xen/interface/elfnote.h>
+
+	__HEAD
+	.code32
+
+/* Entry point for PVH guests */
+ENTRY(pvh_start_xen)
+	cli
+	cld
+
+	mov $_pa(gdt), %eax
+	lgdt (%eax)
+
+	movl $(__BOOT_DS),%eax
+	movl %eax,%ds
+	movl %eax,%es
+	movl %eax,%ss
+
+	/* Stash hvm_start_info */
+	mov $_pa(pvh_start_info), %edi
+	mov %ebx, %esi
+	mov $_pa(pvh_start_info_sz), %ecx
+	mov (%ecx), %ecx
+	rep
+	movsb
+
+	movl $_pa(early_stack_end), %eax
+	movl %eax, %esp
+
+	/* Enable PAE mode */
+	movl %cr4, %eax
+	orl $X86_CR4_PAE, %eax
+	movl %eax, %cr4
+
+#ifdef CONFIG_X86_64
+	/* Enable Long mode */
+	movl $MSR_EFER, %ecx
+	rdmsr
+	btsl $_EFER_LME, %eax
+	wrmsr
+
+	/* Enable pre-constructed page tables */
+	mov $_pa(init_level4_pgt), %eax
+	movl %eax, %cr3
+	movl $(X86_CR0_PG | X86_CR0_PE), %eax
+	movl %eax, %cr0
+
+	/* Jump to 64-bit mode. */
+	pushl $__KERNEL_CS
+	leal _pa(1f), %eax
+	pushl %eax
+	lret
+
+	/* 64-bit entry point */
+	.code64
+1:
+	call xen_prepare_pvh
+
+	/* startup_64 expects boot_params in %rsi */
+	mov $_pa(pvh_bootparams), %rsi
+	movq $_pa(startup_64), %rax
+	jmp *%rax
+
+#else /* CONFIG_X86_64 */
+
+	call setup_pgtable_32
+
+	mov $_pa(initial_page_table), %eax
+	movl %eax, %cr3
+
+	movl %cr0, %eax
+	orl $(X86_CR0_PG | X86_CR0_PE), %eax
+	movl %eax, %cr0
+
+	ljmp $__BOOT_CS,$1f
+1:
+	call xen_prepare_pvh
+	mov $_pa(pvh_bootparams), %esi
+
+	/* startup_32 doesn't expect paging and PAE to be on */
+	ljmp $__BOOT_CS,$_pa(2f)
+2:
+	movl %cr0, %eax
+	andl $~X86_CR0_PG, %eax
+	movl %eax, %cr0
+	movl %cr4, %eax
+	andl $~X86_CR4_PAE, %eax
+	movl %eax, %cr4
+
+	ljmp    $0x10, $_pa(startup_32)
+#endif
+
+	.data
+gdt:
+	.word	gdt_end - gdt
+	.long	_pa(gdt)
+	.word	0
+	.quad	0x0000000000000000 /* NULL descriptor */
+#ifdef CONFIG_X86_64
+	.quad	0x00af9a000000ffff /* __KERNEL_CS */
+#else
+	.quad	0x00cf9a000000ffff /* __KERNEL_CS */
+#endif
+	.quad	0x00cf92000000ffff /* __KERNEL_DS */
+gdt_end:
+
+	.bss
+	.balign 4
+early_stack:
+	.fill 16, 1, 0
+early_stack_end:
+
+	ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,
+	             _ASM_PTR (pvh_start_xen - __START_KERNEL_map))
diff --git a/include/xen/xen.h b/include/xen/xen.h
index d0f9684..ed3f841 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -29,6 +29,11 @@ enum xen_domain_type {
 #define xen_initial_domain()	(0)
 #endif	/* CONFIG_XEN_DOM0 */
 
+#ifdef CONFIG_XEN_PVH
+extern int xen_pvh;
+#define xen_pvh_domain()	(xen_hvm_domain() && xen_pvh)
+#else
 #define xen_pvh_domain()	(0)
+#endif
 
 #endif	/* _XEN_XEN_H */
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1501146 — Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest

On 14/10/16 19:05, Boris Ostrovsky wrote:
> diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
> new file mode 100644
> index 0000000..58c477b
> --- /dev/null
> +++ b/arch/x86/xen/xen-pvh.S
> @@ -0,0 +1,143 @@
> +/*
> + * Copyright C 2016, Oracle and/or its affiliates. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +	.code32
> +	.text
> +#define _pa(x)          ((x) - __START_KERNEL_map)
> +
> +#include <linux/elfnote.h>
> +#include <linux/init.h>
> +#include <linux/linkage.h>
> +#include <asm/segment.h>
> +#include <asm/asm.h>
> +#include <asm/boot.h>
> +#include <asm/processor-flags.h>
> +#include <asm/msr.h>
> +#include <xen/interface/elfnote.h>
> +
> +	__HEAD
> +	.code32

Duplicated .code32

> +
> +/* Entry point for PVH guests */
> +ENTRY(pvh_start_xen)
> +	cli
> +	cld

The ABI states that these will be clear.

> +
> +	mov $_pa(gdt), %eax
> +	lgdt (%eax)

I am fairly sure you can express this without an intermediate in %eax.

> +
> +	movl $(__BOOT_DS),%eax
> +	movl %eax,%ds
> +	movl %eax,%es
> +	movl %eax,%ss
> +
> +	/* Stash hvm_start_info */
> +	mov $_pa(pvh_start_info), %edi
> +	mov %ebx, %esi
> +	mov $_pa(pvh_start_info_sz), %ecx
> +	mov (%ecx), %ecx

No need for an intermediate.

> +	rep
> +	movsb

Surely we can guarentee the size is a multiple of 4? movsl would be better.

> +
> +	movl $_pa(early_stack_end), %eax
> +	movl %eax, %esp

You can mov straight into %esp.

> +
> +	/* Enable PAE mode */
> +	movl %cr4, %eax
> +	orl $X86_CR4_PAE, %eax
> +	movl %eax, %cr4
> +
> +#ifdef CONFIG_X86_64
> +	/* Enable Long mode */
> +	movl $MSR_EFER, %ecx
> +	rdmsr
> +	btsl $_EFER_LME, %eax
> +	wrmsr
> +
> +	/* Enable pre-constructed page tables */
> +	mov $_pa(init_level4_pgt), %eax
> +	movl %eax, %cr3
> +	movl $(X86_CR0_PG | X86_CR0_PE), %eax
> +	movl %eax, %cr0
> +
> +	/* Jump to 64-bit mode. */
> +	pushl $__KERNEL_CS
> +	leal _pa(1f), %eax
> +	pushl %eax
> +	lret

You are still in compat mode, so can ljmp $__KERNEL_CS, $_pa(1f)

> +
> +	/* 64-bit entry point */
> +	.code64
> +1:
> +	call xen_prepare_pvh
> +
> +	/* startup_64 expects boot_params in %rsi */
> +	mov $_pa(pvh_bootparams), %rsi
> +	movq $_pa(startup_64), %rax

You seem to have an inconsistent mix of writing the explicit suffixes
when they aren't required.

> +	jmp *%rax
> +
> +#else /* CONFIG_X86_64 */
> +
> +	call setup_pgtable_32
> +
> +	mov $_pa(initial_page_table), %eax
> +	movl %eax, %cr3
> +
> +	movl %cr0, %eax
> +	orl $(X86_CR0_PG | X86_CR0_PE), %eax
> +	movl %eax, %cr0
> +
> +	ljmp $__BOOT_CS,$1f
> +1:
> +	call xen_prepare_pvh

Why does xen_prepare_pvh need paging?  I can't spot anything which
should need it, and it feels conceptually wrong.

~Andrew

> +	mov $_pa(pvh_bootparams), %esi
> +
> +	/* startup_32 doesn't expect paging and PAE to be on */
> +	ljmp $__BOOT_CS,$_pa(2f)
> +2:
> +	movl %cr0, %eax
> +	andl $~X86_CR0_PG, %eax
> +	movl %eax, %cr0
> +	movl %cr4, %eax
> +	andl $~X86_CR4_PAE, %eax
> +	movl %eax, %cr4
> +
> +	ljmp    $0x10, $_pa(startup_32)
> +#endif
> +
> +	.data
> +gdt:
> +	.word	gdt_end - gdt
> +	.long	_pa(gdt)
> +	.word	0
> +	.quad	0x0000000000000000 /* NULL descriptor */
> +#ifdef CONFIG_X86_64
> +	.quad	0x00af9a000000ffff /* __KERNEL_CS */
> +#else
> +	.quad	0x00cf9a000000ffff /* __KERNEL_CS */
> +#endif
> +	.quad	0x00cf92000000ffff /* __KERNEL_DS */
> +gdt_end:
> +
> +	.bss
> +	.balign 4
> +early_stack:
> +	.fill 16, 1, 0
> +early_stack_end:
> +
> +	ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,
> +	             _ASM_PTR (pvh_start_xen - __START_KERNEL_map))
>

[toc] | [prev] | [next] | [standalone]

#1501152 — Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest

On 10/14/2016 02:38 PM, Andrew Cooper wrote:
>> +	jmp *%rax
>> +
>> +#else /* CONFIG_X86_64 */
>> +
>> +	call setup_pgtable_32
>> +
>> +	mov $_pa(initial_page_table), %eax
>> +	movl %eax, %cr3
>> +
>> +	movl %cr0, %eax
>> +	orl $(X86_CR0_PG | X86_CR0_PE), %eax
>> +	movl %eax, %cr0
>> +
>> +	ljmp $__BOOT_CS,$1f
>> +1:
>> +	call xen_prepare_pvh
> Why does xen_prepare_pvh need paging?  I can't spot anything which
> should need it, and it feels conceptually wrong.

xen_prepare_pvh() deals with virtual addresses. How can we run without paging?

(Also, startup_64, which is where we jump from here in 64-bit mode expects paging to be on).

-boris

[toc] | [prev] | [next] | [standalone]

#1501160 — Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest

On 14/10/16 19:55, Boris Ostrovsky wrote:
> On 10/14/2016 02:38 PM, Andrew Cooper wrote:
>>> +	jmp *%rax
>>> +
>>> +#else /* CONFIG_X86_64 */
>>> +
>>> +	call setup_pgtable_32
>>> +
>>> +	mov $_pa(initial_page_table), %eax
>>> +	movl %eax, %cr3
>>> +
>>> +	movl %cr0, %eax
>>> +	orl $(X86_CR0_PG | X86_CR0_PE), %eax
>>> +	movl %eax, %cr0
>>> +
>>> +	ljmp $__BOOT_CS,$1f
>>> +1:
>>> +	call xen_prepare_pvh
>> Why does xen_prepare_pvh need paging?  I can't spot anything which
>> should need it, and it feels conceptually wrong.
> xen_prepare_pvh() deals with virtual addresses. How can we run without paging?

Ah yes - with a high-half kernel, that way around doesn't work.  Sorry
for the noise - I have been spending too long working with virtual
addresses down round 0, where that specifically can be solved by setting
%ds with a suitable non-zero base.

~Andrew

[toc] | [prev] | [next] | [standalone]

#1501159 — Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest

On Fri, Oct 14, 2016 at 02:05:14PM -0400, Boris Ostrovsky wrote:
> Start PVH guest at XEN_ELFNOTE_PHYS32_ENTRY address. Setup hypercall
> page, initialize boot_params, enable early page tables.
> 
> Since this stub is executed before kernel entry point we cannot use
> variables in .bss which is cleared by kernel. We explicitly place
> variables that are initialized here into .data.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
> ---
>  arch/x86/xen/Kconfig     |   2 +-
>  arch/x86/xen/Makefile    |   1 +
>  arch/x86/xen/enlighten.c |  87 +++++++++++++++++++++++++++-
>  arch/x86/xen/xen-pvh.S   | 143 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/xen/xen.h        |   5 ++
>  5 files changed, 236 insertions(+), 2 deletions(-)
>  create mode 100644 arch/x86/xen/xen-pvh.S
> 
> diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
> index c7b15f3..76b6dbd 100644
> --- a/arch/x86/xen/Kconfig
> +++ b/arch/x86/xen/Kconfig
> @@ -53,5 +53,5 @@ config XEN_DEBUG_FS
>  
>  config XEN_PVH
>  	bool "Support for running as a PVH guest"
> -	depends on X86_64 && XEN && XEN_PVHVM
> +	depends on XEN && XEN_PVHVM && ACPI
>  	def_bool n
> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
> index e47e527..cb0164a 100644
> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_XEN_DEBUG_FS)	+= debugfs.o
>  obj-$(CONFIG_XEN_DOM0)		+= vga.o
>  obj-$(CONFIG_SWIOTLB_XEN)	+= pci-swiotlb-xen.o
>  obj-$(CONFIG_XEN_EFI)		+= efi.o
> +obj-$(CONFIG_XEN_PVH)	 	+= xen-pvh.o
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index dc4ed0c..d38d568 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -45,6 +45,7 @@
>  #include <xen/interface/memory.h>
>  #include <xen/interface/nmi.h>
>  #include <xen/interface/xen-mca.h>
> +#include <xen/interface/hvm/start_info.h>
>  #include <xen/features.h>
>  #include <xen/page.h>
>  #include <xen/hvm.h>
> @@ -121,7 +122,8 @@
>  DEFINE_PER_CPU(uint32_t, xen_vcpu_id);
>  EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
>  
> -enum xen_domain_type xen_domain_type = XEN_NATIVE;
> +enum xen_domain_type xen_domain_type
> +	__attribute__((section(".data"))) = XEN_NATIVE;
>  EXPORT_SYMBOL_GPL(xen_domain_type);
>  
>  unsigned long *machine_to_phys_mapping = (void *)MACH2PHYS_VIRT_START;
> @@ -176,6 +178,17 @@ struct tls_descs {
>   */
>  static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>  
> +#ifdef CONFIG_XEN_PVH
> +/*
> + * PVH variables. These need to live in data segment since they are
> + * initialized before startup_{32|64}, which clear .bss, are invoked.
> + */
> +int xen_pvh __attribute__((section(".data"))) = 0;

unsigned int?
> +struct hvm_start_info pvh_start_info __attribute__((section(".data")));
> +uint pvh_start_info_sz = sizeof(pvh_start_info);

unsigned int please. Typedefs in Linux are frowned upon.

> +struct boot_params pvh_bootparams __attribute__((section(".data")));
> +#endif
> +
>  static void clamp_max_cpus(void)
>  {
>  #ifdef CONFIG_SMP
> @@ -1669,6 +1682,78 @@ asmlinkage __visible void __init xen_start_kernel(void)
>  #endif
>  }
>  
> +#ifdef CONFIG_XEN_PVH
> +static void __init init_pvh_bootparams(void)
> +{
> +	struct xen_memory_map memmap;
> +	int i;

unsigned int?
> +
> +	memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
> +
> +	memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_map);
> +	set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_map);
> +	if (HYPERVISOR_memory_op(XENMEM_memory_map, &memmap)) {
> +		xen_raw_console_write("XENMEM_memory_map failed\n");

Should we print the error value at least?
> +		BUG();
> +	}
> +
> +	pvh_bootparams.e820_map[memmap.nr_entries].addr =
> +		ISA_START_ADDRESS;

What if nr_entries is 128? Should we double-check for that?

> +	pvh_bootparams.e820_map[memmap.nr_entries].size =
> +		ISA_END_ADDRESS - ISA_START_ADDRESS;
> +	pvh_bootparams.e820_map[memmap.nr_entries++].type =
> +		E820_RESERVED;
> +
> +	sanitize_e820_map(pvh_bootparams.e820_map,
> +			  ARRAY_SIZE(pvh_bootparams.e820_map),
> +			  &memmap.nr_entries); 
> +
> +	pvh_bootparams.e820_entries = memmap.nr_entries;
> +	for (i = 0; i < pvh_bootparams.e820_entries; i++)
> +		e820_add_region(pvh_bootparams.e820_map[i].addr,
> +				pvh_bootparams.e820_map[i].size,
> +				pvh_bootparams.e820_map[i].type);
> +
> +	pvh_bootparams.hdr.cmd_line_ptr =
> +		pvh_start_info.cmdline_paddr;
> +
> +	/* The first module is always ramdisk */

Could you add an period at end please?
> +	if (pvh_start_info.nr_modules) {
> +		struct hvm_modlist_entry *modaddr =
> +			__va(pvh_start_info.modlist_paddr);
> +		pvh_bootparams.hdr.ramdisk_image = modaddr->paddr;
> +		pvh_bootparams.hdr.ramdisk_size = modaddr->size;
> +	}
> +
> +	/*
> +	 * See Documentation/x86/boot.txt.
> +	 *
> +	 * Version 2.12 supports Xen entry point but we will use default x86/PC
> +	 * environment (i.e. hardware_subarch 0).
> +	 */
> +	pvh_bootparams.hdr.version = 0x212;
> +	pvh_bootparams.hdr.type_of_loader = (9 << 4) | 0; /* Xen loader */
> +}
> +
> +/*
> + * This routine (and those that it might call) should not use
> + * anything that lives in .bss since that segment will be cleared later

And maybe one here too?
> + */
> +void __init xen_prepare_pvh(void)
> +{
> +	u32 eax, ecx, edx, msr;

msr = 0 ?
> +	u64 pfn;
> +
> +	xen_pvh = 1;
> +
> +	cpuid(xen_cpuid_base() + 2, &eax, &msr, &ecx, &edx);

cpuid_ebx ? And that way you don't have have ecx and edx?
> +	pfn = __pa(hypercall_page);
> +	wrmsr_safe(msr, (u32)pfn, (u32)(pfn >> 32));
> +
> +	init_pvh_bootparams();
> +}
> +#endif
> +
>  void __ref xen_hvm_init_shared_info(void)
>  {
>  	int cpu;
> diff --git a/arch/x86/xen/xen-pvh.S b/arch/x86/xen/xen-pvh.S
> new file mode 100644
> index 0000000..58c477b
> --- /dev/null
> +++ b/arch/x86/xen/xen-pvh.S
> @@ -0,0 +1,143 @@
> +/*
> + * Copyright C 2016, Oracle and/or its affiliates. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +	.code32
> +	.text
> +#define _pa(x)          ((x) - __START_KERNEL_map)
> +
> +#include <linux/elfnote.h>
> +#include <linux/init.h>
> +#include <linux/linkage.h>
> +#include <asm/segment.h>
> +#include <asm/asm.h>
> +#include <asm/boot.h>
> +#include <asm/processor-flags.h>
> +#include <asm/msr.h>
> +#include <xen/interface/elfnote.h>
> +
> +	__HEAD
> +	.code32
> +
> +/* Entry point for PVH guests */
> +ENTRY(pvh_start_xen)

You are missing the ENDPROC macro at the end.

> +	cli
> +	cld
> +
> +	mov $_pa(gdt), %eax
> +	lgdt (%eax)
> +
> +	movl $(__BOOT_DS),%eax
> +	movl %eax,%ds
> +	movl %eax,%es
> +	movl %eax,%ss
> +
> +	/* Stash hvm_start_info */
> +	mov $_pa(pvh_start_info), %edi
> +	mov %ebx, %esi

Should we derference the first byte or such to check for the magic
string? Actually I am not even seeing the check in the C code?

> +	mov $_pa(pvh_start_info_sz), %ecx
> +	mov (%ecx), %ecx
> +	rep
> +	movsb
> +
> +	movl $_pa(early_stack_end), %eax
> +	movl %eax, %esp
> +
> +	/* Enable PAE mode */

Periods are nice! Truly!

> +	movl %cr4, %eax
> +	orl $X86_CR4_PAE, %eax
> +	movl %eax, %cr4
> +
> +#ifdef CONFIG_X86_64
> +	/* Enable Long mode */

:-) I think you know what I am going to say here.

> +	movl $MSR_EFER, %ecx
> +	rdmsr
> +	btsl $_EFER_LME, %eax
> +	wrmsr
> +
> +	/* Enable pre-constructed page tables */

And here.

> +	mov $_pa(init_level4_pgt), %eax
> +	movl %eax, %cr3
> +	movl $(X86_CR0_PG | X86_CR0_PE), %eax
> +	movl %eax, %cr0
> +
> +	/* Jump to 64-bit mode. */
> +	pushl $__KERNEL_CS
> +	leal _pa(1f), %eax
> +	pushl %eax
> +	lret
> +
> +	/* 64-bit entry point */

And right here.
> +	.code64
> +1:
> +	call xen_prepare_pvh
> +
> +	/* startup_64 expects boot_params in %rsi */

..
> +	mov $_pa(pvh_bootparams), %rsi
> +	movq $_pa(startup_64), %rax
> +	jmp *%rax
> +
> +#else /* CONFIG_X86_64 */
> +
> +	call setup_pgtable_32
> +
> +	mov $_pa(initial_page_table), %eax
> +	movl %eax, %cr3
> +
> +	movl %cr0, %eax
> +	orl $(X86_CR0_PG | X86_CR0_PE), %eax
> +	movl %eax, %cr0
> +
> +	ljmp $__BOOT_CS,$1f
> +1:
> +	call xen_prepare_pvh
> +	mov $_pa(pvh_bootparams), %esi
> +
> +	/* startup_32 doesn't expect paging and PAE to be on */

Should 'startup_32' be documented with this?


> +	ljmp $__BOOT_CS,$_pa(2f)
> +2:
> +	movl %cr0, %eax
> +	andl $~X86_CR0_PG, %eax
> +	movl %eax, %cr0
> +	movl %cr4, %eax
> +	andl $~X86_CR4_PAE, %eax
> +	movl %eax, %cr4
> +
> +	ljmp    $0x10, $_pa(startup_32)
> +#endif
> +
> +	.data
> +gdt:
> +	.word	gdt_end - gdt
> +	.long	_pa(gdt)
> +	.word	0
> +	.quad	0x0000000000000000 /* NULL descriptor */
> +#ifdef CONFIG_X86_64
> +	.quad	0x00af9a000000ffff /* __KERNEL_CS */
> +#else
> +	.quad	0x00cf9a000000ffff /* __KERNEL_CS */
> +#endif
> +	.quad	0x00cf92000000ffff /* __KERNEL_DS */
> +gdt_end:
> +
> +	.bss
> +	.balign 4
> +early_stack:
> +	.fill 16, 1, 0
> +early_stack_end:
> +
> +	ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,
> +	             _ASM_PTR (pvh_start_xen - __START_KERNEL_map))
> diff --git a/include/xen/xen.h b/include/xen/xen.h
> index d0f9684..ed3f841 100644
> --- a/include/xen/xen.h
> +++ b/include/xen/xen.h
> @@ -29,6 +29,11 @@ enum xen_domain_type {
>  #define xen_initial_domain()	(0)
>  #endif	/* CONFIG_XEN_DOM0 */
>  
> +#ifdef CONFIG_XEN_PVH
> +extern int xen_pvh;
> +#define xen_pvh_domain()	(xen_hvm_domain() && xen_pvh)
> +#else
>  #define xen_pvh_domain()	(0)
> +#endif
>  
>  #endif	/* _XEN_XEN_H */
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

[toc] | [prev] | [next] | [standalone]

#1501168 — Re: [Xen-devel] [PATCH 4/8] xen/pvh: Bootstrap PVH guest

On 10/14/2016 03:14 PM, Konrad Rzeszutek Wilk wrote:
>
>> +
>> +	memset(&pvh_bootparams, 0, sizeof(pvh_bootparams));
>> +
>> +	memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_map);
>> +	set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_map);
>> +	if (HYPERVISOR_memory_op(XENMEM_memory_map, &memmap)) {
>> +		xen_raw_console_write("XENMEM_memory_map failed\n");
> Should we print the error value at least?

I will have to check again but IIRC there was something about not being
able to format strings properly this early. But if we can --- sure.

>> +		BUG();
>> +	}
>> +
>> +	pvh_bootparams.e820_map[memmap.nr_entries].addr =
>> +		ISA_START_ADDRESS;
> What if nr_entries is 128? Should we double-check for that?
>

OK.



>> + */
>> +void __init xen_prepare_pvh(void)
>> +{
>> +	u32 eax, ecx, edx, msr;
> msr = 0 ?

Won't cpuid() (or cpuid_ebx()) overwrite it anyway?

>> +	u64 pfn;
>> +
>> +	xen_pvh = 1;
>> +
>> +	cpuid(xen_cpuid_base() + 2, &eax, &msr, &ecx, &edx);
> cpuid_ebx ? And that way you don't have have ecx and edx?



>> +	cli
>> +	cld
>> +
>> +	mov $_pa(gdt), %eax
>> +	lgdt (%eax)
>> +
>> +	movl $(__BOOT_DS),%eax
>> +	movl %eax,%ds
>> +	movl %eax,%es
>> +	movl %eax,%ss
>> +
>> +	/* Stash hvm_start_info */
>> +	mov $_pa(pvh_start_info), %edi
>> +	mov %ebx, %esi
> Should we derference the first byte or such to check for the magic
> string? Actually I am not even seeing the check in the C code?


Yes, good idea.


>> +	.code64
>> +1:
>> +	call xen_prepare_pvh
>> +
>> +	/* startup_64 expects boot_params in %rsi */
> ..
>> +	mov $_pa(pvh_bootparams), %rsi
>> +	movq $_pa(startup_64), %rax
>> +	jmp *%rax
>> +
>> +#else /* CONFIG_X86_64 */
>> +
>> +	call setup_pgtable_32
>> +
>> +	mov $_pa(initial_page_table), %eax
>> +	movl %eax, %cr3
>> +
>> +	movl %cr0, %eax
>> +	orl $(X86_CR0_PG | X86_CR0_PE), %eax
>> +	movl %eax, %cr0
>> +
>> +	ljmp $__BOOT_CS,$1f
>> +1:
>> +	call xen_prepare_pvh
>> +	mov $_pa(pvh_bootparams), %esi
>> +
>> +	/* startup_32 doesn't expect paging and PAE to be on */
> Should 'startup_32' be documented with this?

It is documented in Documentation/x86/boot.txt and in the startup_64 code.


-boris

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web