[PATCH for-4.11] x86/dom0: add extra RAM regions as RESERVED for PVH memory map

[PATCH for-4.11] x86/dom0: add extra RAM regions as RESERVED for PVH memory map

[Roger Pau Monne]

When running as PVH Dom0 the native memory map is used in order to
craft a tailored memory map for Dom0 taking into account it's memory
limit.

Dom0 memory is always going to be smaller than the total amount
of memory present on the host, so in order to prevent Dom0 from
relocating PCI BARs over RAM regions mark all the RAM regions not
available to Dom0 as RESERVED in the memory map.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
---
NB: I haven't seen any system where Dom0 would relocate the BARs over
RAM regions, but AFAICT given the current memory map provided to Dom0
this is a possibility that should be avoided.

I also think this should be included in the 4.11 release because it
only modifies PVH Dom0 builder code, and that's still experimental, so
there's no chance of breaking a supported feature.
---
 xen/arch/x86/hvm/dom0_build.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index b237508072..9a441ab4cf 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -314,8 +314,10 @@ static __init void pvh_setup_e820(struct domain *d, unsigned long nr_pages)
 
     /*
      * Craft the e820 memory map for Dom0 based on the hardware e820 map.
+     * Add an extra entry in case we have to split a RAM entry into a RAM and a
+     * RESERVED one in order to truncate it.
      */
-    d->arch.e820 = xzalloc_array(struct e820entry, e820.nr_map);
+    d->arch.e820 = xzalloc_array(struct e820entry, e820.nr_map + 1);
     if ( !d->arch.e820 )
         panic("Unable to allocate memory for Dom0 e820 map");
     entry_guest = d->arch.e820;
@@ -323,19 +325,20 @@ static __init void pvh_setup_e820(struct domain *d, unsigned long nr_pages)
     /* Clamp e820 memory map to match the memory assigned to Dom0 */
     for ( i = 0, entry = e820.map; i < e820.nr_map; i++, entry++ )
     {
+        *entry_guest = *entry;
+
         if ( entry->type != E820_RAM )
-        {
-            *entry_guest = *entry;
             goto next;
-        }
 
         if ( nr_pages == cur_pages )
         {
             /*
-             * We already have all the assigned memory,
-             * skip this entry
+             * We already have all the requested memory, turn this RAM region
+             * into a RESERVED region in order to prevent Dom0 from placing
+             * BARs in this area.
              */
-            continue;
+            entry_guest->type = E820_RESERVED;
+            goto next;
         }
 
         /*
@@ -358,6 +361,12 @@ static __init void pvh_setup_e820(struct domain *d, unsigned long nr_pages)
         {
             /* Truncate region */
             entry_guest->size = (nr_pages - cur_pages) << PAGE_SHIFT;
+            /* Add the remaining of the RAM region as RESERVED. */
+            entry_guest++;
+            d->arch.nr_e820++;
+            entry_guest->type = E820_RESERVED;
+            entry_guest->addr = start + ((nr_pages - cur_pages) << PAGE_SHIFT);
+            entry_guest->size = end - entry_guest->addr;
             cur_pages = nr_pages;
         }
         else
@@ -367,9 +376,9 @@ static __init void pvh_setup_e820(struct domain *d, unsigned long nr_pages)
  next:
         d->arch.nr_e820++;
         entry_guest++;
+        ASSERT(d->arch.nr_e820 <= e820.nr_map + 1);
     }
     ASSERT(cur_pages == nr_pages);
-    ASSERT(d->arch.nr_e820 <= e820.nr_map);
 }
 
 static int __init pvh_setup_p2m(struct domain *d)
-- 
2.17.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.11] x86/dom0: add extra RAM regions as RESERVED for PVH memory map

[Alexey G]

On Thu, 3 May 2018 10:56:40 +0100
Roger Pau Monne <roger.pau@citrix.com> wrote:

>When running as PVH Dom0 the native memory map is used in order to
>craft a tailored memory map for Dom0 taking into account it's memory
>limit.
>
>Dom0 memory is always going to be smaller than the total amount
>of memory present on the host, so in order to prevent Dom0 from
>relocating PCI BARs over RAM regions mark all the RAM regions not
>available to Dom0 as RESERVED in the memory map.
>---
>NB: I haven't seen any system where Dom0 would relocate the BARs over
>RAM regions, but AFAICT given the current memory map provided to Dom0
>this is a possibility that should be avoided.

Guest OSes typically use information from ACPI to learn where PCI
BARs can (or cannot) be relocated.
AFAIK Linux kernel discard this information only if being told
"pci=nocrs", so the E820 approach should be extra checked if it will
actually override ACPI-provided PCI information in all possible
cases. Not sure how DSDT is used on PVH though.

>I also think this should be included in the 4.11 release because it
>only modifies PVH Dom0 builder code, and that's still experimental, so
>there's no chance of breaking a supported feature.
>---
> xen/arch/x86/hvm/dom0_build.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
>diff --git a/xen/arch/x86/hvm/dom0_build.c
>b/xen/arch/x86/hvm/dom0_build.c index b237508072..9a441ab4cf 100644
>--- a/xen/arch/x86/hvm/dom0_build.c
>+++ b/xen/arch/x86/hvm/dom0_build.c
>@@ -314,8 +314,10 @@ static __init void pvh_setup_e820(struct domain
>*d, unsigned long nr_pages)
> 
>     /*
>      * Craft the e820 memory map for Dom0 based on the hardware e820
> map.
>+     * Add an extra entry in case we have to split a RAM entry into a
>RAM and a
>+     * RESERVED one in order to truncate it.
>      */
>-    d->arch.e820 = xzalloc_array(struct e820entry, e820.nr_map);
>+    d->arch.e820 = xzalloc_array(struct e820entry, e820.nr_map + 1);
>     if ( !d->arch.e820 )
>         panic("Unable to allocate memory for Dom0 e820 map");
>     entry_guest = d->arch.e820;
>@@ -323,19 +325,20 @@ static __init void pvh_setup_e820(struct domain
>*d, unsigned long nr_pages)
>     /* Clamp e820 memory map to match the memory assigned to Dom0 */
>     for ( i = 0, entry = e820.map; i < e820.nr_map; i++, entry++ )
>     {
>+        *entry_guest = *entry;
>+
>         if ( entry->type != E820_RAM )
>-        {
>-            *entry_guest = *entry;
>             goto next;
>-        }
> 
>         if ( nr_pages == cur_pages )
>         {
>             /*
>-             * We already have all the assigned memory,
>-             * skip this entry
>+             * We already have all the requested memory, turn this
>RAM region
>+             * into a RESERVED region in order to prevent Dom0 from
>placing
>+             * BARs in this area.
>              */
>-            continue;
>+            entry_guest->type = E820_RESERVED;
>+            goto next;
>         }
> 
>         /*
>@@ -358,6 +361,12 @@ static __init void pvh_setup_e820(struct domain
>*d, unsigned long nr_pages)
>         {
>             /* Truncate region */
>             entry_guest->size = (nr_pages - cur_pages) << PAGE_SHIFT;
>+            /* Add the remaining of the RAM region as RESERVED. */
>+            entry_guest++;
>+            d->arch.nr_e820++;
>+            entry_guest->type = E820_RESERVED;
>+            entry_guest->addr = start + ((nr_pages - cur_pages) <<
>PAGE_SHIFT);
>+            entry_guest->size = end - entry_guest->addr;
>             cur_pages = nr_pages;
>         }
>         else
>@@ -367,9 +376,9 @@ static __init void pvh_setup_e820(struct domain
>*d, unsigned long nr_pages)
>  next:
>         d->arch.nr_e820++;
>         entry_guest++;
>+        ASSERT(d->arch.nr_e820 <= e820.nr_map + 1);
>     }
>     ASSERT(cur_pages == nr_pages);
>-    ASSERT(d->arch.nr_e820 <= e820.nr_map);
> }
> 
> static int __init pvh_setup_p2m(struct domain *d)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.11] x86/dom0: add extra RAM regions as RESERVED for PVH memory map

[Roger Pau Monné]

On Thu, May 03, 2018 at 08:55:14PM +1000, Alexey G wrote:
> On Thu, 3 May 2018 10:56:40 +0100
> Roger Pau Monne <roger.pau@citrix.com> wrote:
> 
> >When running as PVH Dom0 the native memory map is used in order to
> >craft a tailored memory map for Dom0 taking into account it's memory
> >limit.
> >
> >Dom0 memory is always going to be smaller than the total amount
> >of memory present on the host, so in order to prevent Dom0 from
> >relocating PCI BARs over RAM regions mark all the RAM regions not
> >available to Dom0 as RESERVED in the memory map.
> >---
> >NB: I haven't seen any system where Dom0 would relocate the BARs over
> >RAM regions, but AFAICT given the current memory map provided to Dom0
> >this is a possibility that should be avoided.
> 
> Guest OSes typically use information from ACPI to learn where PCI
> BARs can (or cannot) be relocated.

I think it's better to be safe than sorry, so IMO the host RAM regions
should be added to the memory map as RESERVED.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.11] x86/dom0: add extra RAM regions as RESERVED for PVH memory map

[Jan Beulich]

>>> On 03.05.18 at 11:56, <roger.pau@citrix.com> wrote:
> When running as PVH Dom0 the native memory map is used in order to
> craft a tailored memory map for Dom0 taking into account it's memory
> limit.
> 
> Dom0 memory is always going to be smaller than the total amount
> of memory present on the host, so in order to prevent Dom0 from
> relocating PCI BARs over RAM regions mark all the RAM regions not
> available to Dom0 as RESERVED in the memory map.

I'm basically fine with this change, but I think I'd prefer UNUSABLE over
RESERVED.

> I also think this should be included in the 4.11 release because it
> only modifies PVH Dom0 builder code, and that's still experimental, so
> there's no chance of breaking a supported feature.

This can equally be taken as a reason why there's no point in it being
put in for 4.11. But that's not to say I'm opposed to doing so, it's just
that I consider the argument questionable.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.11] x86/dom0: add extra RAM regions as RESERVED for PVH memory map

[Alexey G]

On Thu, 3 May 2018 12:15:18 +0100
Roger Pau Monné <roger.pau@citrix.com> wrote:

>On Thu, May 03, 2018 at 08:55:14PM +1000, Alexey G wrote:
>> On Thu, 3 May 2018 10:56:40 +0100
>> Roger Pau Monne <roger.pau@citrix.com> wrote:
>>   
>> >When running as PVH Dom0 the native memory map is used in order to
>> >craft a tailored memory map for Dom0 taking into account it's memory
>> >limit.
>> >
>> >Dom0 memory is always going to be smaller than the total amount
>> >of memory present on the host, so in order to prevent Dom0 from
>> >relocating PCI BARs over RAM regions mark all the RAM regions not
>> >available to Dom0 as RESERVED in the memory map.
>> >---
>> >NB: I haven't seen any system where Dom0 would relocate the BARs
>> >over RAM regions, but AFAICT given the current memory map provided
>> >to Dom0 this is a possibility that should be avoided.  
>> 
>> Guest OSes typically use information from ACPI to learn where PCI
>> BARs can (or cannot) be relocated.  
>
>I think it's better to be safe than sorry, so IMO the host RAM regions
>should be added to the memory map as RESERVED.
>
>Roger.

I assume host's DSDT passed through to PVH Dom0 as is? In this case
Dom0 will see PCI holes matching those of the host and shouldn't make
any attempts to place BARs outside provided PCI holes (except "nocrs"
given).

As long as Dom0 P2M map prevent using these host ranges we shouldn't
worry if it is marked as reserved in e820 map I think. It's kinda
excessive information for Dom0 -- knowing about host RAM ranges which
he cannot touch anyway due to lack of corresponding p2m mappings.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.11] x86/dom0: add extra RAM regions as RESERVED for PVH memory map

[Roger Pau Monné]

On Thu, May 03, 2018 at 05:46:27AM -0600, Jan Beulich wrote:
> >>> On 03.05.18 at 11:56, <roger.pau@citrix.com> wrote:
> > When running as PVH Dom0 the native memory map is used in order to
> > craft a tailored memory map for Dom0 taking into account it's memory
> > limit.
> > 
> > Dom0 memory is always going to be smaller than the total amount
> > of memory present on the host, so in order to prevent Dom0 from
> > relocating PCI BARs over RAM regions mark all the RAM regions not
> > available to Dom0 as RESERVED in the memory map.
> 
> I'm basically fine with this change, but I think I'd prefer UNUSABLE over
> RESERVED.

OK, I've looked at the definitions from the ACPI spec and I think both
are suitable in order to accomplish the goal. So I'm fine with sending
a v2 with UNUSABLE instead of RESERVED.

> > I also think this should be included in the 4.11 release because it
> > only modifies PVH Dom0 builder code, and that's still experimental, so
> > there's no chance of breaking a supported feature.
> 
> This can equally be taken as a reason why there's no point in it being
> put in for 4.11. But that's not to say I'm opposed to doing so, it's just
> that I consider the argument questionable.

Well, it *might* make a Dom0 PVH work in cases it wouldn't, but since
it's experimental anyone playing with PVH Dom0 should probably be
using staging already.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.11] x86/dom0: add extra RAM regions as RESERVED for PVH memory map

[Roger Pau Monné]

On Thu, May 03, 2018 at 10:02:47PM +1000, Alexey G wrote:
> On Thu, 3 May 2018 12:15:18 +0100
> Roger Pau Monné <roger.pau@citrix.com> wrote:
> 
> >On Thu, May 03, 2018 at 08:55:14PM +1000, Alexey G wrote:
> >> On Thu, 3 May 2018 10:56:40 +0100
> >> Roger Pau Monne <roger.pau@citrix.com> wrote:
> >>   
> >> >When running as PVH Dom0 the native memory map is used in order to
> >> >craft a tailored memory map for Dom0 taking into account it's memory
> >> >limit.
> >> >
> >> >Dom0 memory is always going to be smaller than the total amount
> >> >of memory present on the host, so in order to prevent Dom0 from
> >> >relocating PCI BARs over RAM regions mark all the RAM regions not
> >> >available to Dom0 as RESERVED in the memory map.
> >> >---
> >> >NB: I haven't seen any system where Dom0 would relocate the BARs
> >> >over RAM regions, but AFAICT given the current memory map provided
> >> >to Dom0 this is a possibility that should be avoided.  
> >> 
> >> Guest OSes typically use information from ACPI to learn where PCI
> >> BARs can (or cannot) be relocated.  
> >
> >I think it's better to be safe than sorry, so IMO the host RAM regions
> >should be added to the memory map as RESERVED.
> >
> >Roger.
> 
> I assume host's DSDT passed through to PVH Dom0 as is? In this case
> Dom0 will see PCI holes matching those of the host and shouldn't make
> any attempts to place BARs outside provided PCI holes (except "nocrs"
> given).
> 
> As long as Dom0 P2M map prevent using these host ranges we shouldn't
> worry if it is marked as reserved in e820 map I think. It's kinda
> excessive information for Dom0 -- knowing about host RAM ranges which
> he cannot touch anyway due to lack of corresponding p2m mappings.

Dom0 could attempt to relocate a BAR over a RAM region and Xen won't
prevent it, because Dom0 is trusted. The same could happen with a PV
Dom0, but in the PV case Dom0 is provided with the unmodified host
memory map.

I don't see how providing this UNUSABLE/RESERVED ranges is going to
cause any issues to Dom0, so I think we should just do it.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.11] x86/dom0: add extra RAM regions as RESERVED for PVH memory map

[Alexey G]

On Thu, 3 May 2018 15:02:36 +0100
Roger Pau Monné <roger.pau@citrix.com> wrote:

>On Thu, May 03, 2018 at 10:02:47PM +1000, Alexey G wrote:
>> On Thu, 3 May 2018 12:15:18 +0100
>> Roger Pau Monné <roger.pau@citrix.com> wrote:
>>   
>> >On Thu, May 03, 2018 at 08:55:14PM +1000, Alexey G wrote:  
>> >> On Thu, 3 May 2018 10:56:40 +0100
>> >> Roger Pau Monne <roger.pau@citrix.com> wrote:
>> >>     
>> >> >When running as PVH Dom0 the native memory map is used in order
>> >> >to craft a tailored memory map for Dom0 taking into account it's
>> >> >memory limit.
>> >> >
>> >> >Dom0 memory is always going to be smaller than the total amount
>> >> >of memory present on the host, so in order to prevent Dom0 from
>> >> >relocating PCI BARs over RAM regions mark all the RAM regions not
>> >> >available to Dom0 as RESERVED in the memory map.
>> >> >---
>> >> >NB: I haven't seen any system where Dom0 would relocate the BARs
>> >> >over RAM regions, but AFAICT given the current memory map
>> >> >provided to Dom0 this is a possibility that should be
>> >> >avoided.    
>> >> 
>> >> Guest OSes typically use information from ACPI to learn where PCI
>> >> BARs can (or cannot) be relocated.    
>> >
>> >I think it's better to be safe than sorry, so IMO the host RAM
>> >regions should be added to the memory map as RESERVED.
>> >
>> >Roger.  
>> 
>> I assume host's DSDT passed through to PVH Dom0 as is? In this case
>> Dom0 will see PCI holes matching those of the host and shouldn't make
>> any attempts to place BARs outside provided PCI holes (except "nocrs"
>> given).
>> 
>> As long as Dom0 P2M map prevent using these host ranges we shouldn't
>> worry if it is marked as reserved in e820 map I think. It's kinda
>> excessive information for Dom0 -- knowing about host RAM ranges which
>> he cannot touch anyway due to lack of corresponding p2m mappings.  
>
>Dom0 could attempt to relocate a BAR over a RAM region and Xen won't
>prevent it, because Dom0 is trusted. The same could happen with a PV
>Dom0, but in the PV case Dom0 is provided with the unmodified host
>memory map.
>

In worst case the physical device won't work if it will be relocated
outside the host MMIO hole -- the system won't decode accesses to it.
If the reason just to tell dom0 allowable limits where PCI MMIO BARs can
be safely relocated without breaking their decoding -- then _CRS should
have priority over e820 map as it is the primary source of MMIO hole
information on ACPI-capable systems. Anyway, this patch might be useful
in situations like running kernel with nocrs/noacpi to workaround some
platform issues.

Other than that, there should be no critical issues due to PCI BAR
relocation over host RAM. Dom0 can't just create some arbitrary mapping
to host memory if it doesn't belong to any running domain. Even
attempting to map host RAM to dom0 as MMIO won't work IIRC -- p2m/mm
code will prohibit this. Don't remember the exact reason, but AFAIR it
should complain about wannabe-MMIO ranges not belonging to dom_io,
something like that. So there will be no corresponding p2m mappings
available for dom0 to gain access to host RAM ranges where relocated
BARs point, even if dom0 can control these BARs.

>I don't see how providing this UNUSABLE/RESERVED ranges is going to
>cause any issues to Dom0, so I think we should just do it.
>
>Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel