Re: [Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH

[Julien Grall]

Hi,

On 12/06/2017 11:47 AM, Roger Pau Monné wrote:

On Wed, Dec 06, 2017 at 12:22:00PM +0100, Juergen Gross wrote:

On 06/12/17 10:53, Julien Grall wrote:

Hi Juergen,

On 12/05/2017 04:19 PM, Juergen Gross wrote:

On 05/12/17 16:23, Julien Grall wrote:

Hi Juergen,

On 04/12/17 15:49, Juergen Gross wrote:

On 21/11/17 12:06, Juergen Gross wrote:

The "special pages" for PVH guests include the frames for console and
Xenstore ring buffers. Those have to be marked as "Reserved" in the
guest's E820 map, as otherwise conflicts might arise later e.g. when
hotplugging memory into the guest.

Signed-off-by: Juergen Gross 
---
This is a bugfix for PVH guests. Please consider for 4.10.


Ping?


I was waiting an ack from tools maintainers before looking for a release
perspective.

I would recommend to tag your patch is 4.10 to help reviewers prioritize
review on your patch. I have done it now.

I am looking at releasing Xen 4.10 in the next few days. Can you explain
the pros/cons of this patch?


Pros: PVH guests with 4GB of memory or more will work. :-)


They never worked before? Or is it a regression? If it is a regression
when did it appear?


Hmm, seems we are lucky: Linux kernel will not try to map any memory
there (I just tested it). So we don't need that patch in 4.10 for Linux
running as PVH guest. Not sure about BSD, though.


I haven't yet committed any PVHv2 support to FreeBSD, but in any case
I always avoid using memory below 4GB just in case...


I will defer this patch post 4.10. We can add a release note if you 
think it is worth it.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH

[Roger Pau Monné]

On Wed, Dec 06, 2017 at 12:22:00PM +0100, Juergen Gross wrote:
> On 06/12/17 10:53, Julien Grall wrote:
> > Hi Juergen,
> > 
> > On 12/05/2017 04:19 PM, Juergen Gross wrote:
> >> On 05/12/17 16:23, Julien Grall wrote:
> >>> Hi Juergen,
> >>>
> >>> On 04/12/17 15:49, Juergen Gross wrote:
>  On 21/11/17 12:06, Juergen Gross wrote:
> > The "special pages" for PVH guests include the frames for console and
> > Xenstore ring buffers. Those have to be marked as "Reserved" in the
> > guest's E820 map, as otherwise conflicts might arise later e.g. when
> > hotplugging memory into the guest.
> >
> > Signed-off-by: Juergen Gross 
> > ---
> > This is a bugfix for PVH guests. Please consider for 4.10.
> 
>  Ping?
> >>>
> >>> I was waiting an ack from tools maintainers before looking for a release
> >>> perspective.
> >>>
> >>> I would recommend to tag your patch is 4.10 to help reviewers prioritize
> >>> review on your patch. I have done it now.
> >>>
> >>> I am looking at releasing Xen 4.10 in the next few days. Can you explain
> >>> the pros/cons of this patch?
> >>
> >> Pros: PVH guests with 4GB of memory or more will work. :-)
> > 
> > They never worked before? Or is it a regression? If it is a regression
> > when did it appear?
> 
> Hmm, seems we are lucky: Linux kernel will not try to map any memory
> there (I just tested it). So we don't need that patch in 4.10 for Linux
> running as PVH guest. Not sure about BSD, though.

I haven't yet committed any PVHv2 support to FreeBSD, but in any case
I always avoid using memory below 4GB just in case...

Roger.

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

Re: [Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH

[Juergen Gross]

On 06/12/17 10:53, Julien Grall wrote:
> Hi Juergen,
> 
> On 12/05/2017 04:19 PM, Juergen Gross wrote:
>> On 05/12/17 16:23, Julien Grall wrote:
>>> Hi Juergen,
>>>
>>> On 04/12/17 15:49, Juergen Gross wrote:
 On 21/11/17 12:06, Juergen Gross wrote:
> The "special pages" for PVH guests include the frames for console and
> Xenstore ring buffers. Those have to be marked as "Reserved" in the
> guest's E820 map, as otherwise conflicts might arise later e.g. when
> hotplugging memory into the guest.
>
> Signed-off-by: Juergen Gross 
> ---
> This is a bugfix for PVH guests. Please consider for 4.10.

 Ping?
>>>
>>> I was waiting an ack from tools maintainers before looking for a release
>>> perspective.
>>>
>>> I would recommend to tag your patch is 4.10 to help reviewers prioritize
>>> review on your patch. I have done it now.
>>>
>>> I am looking at releasing Xen 4.10 in the next few days. Can you explain
>>> the pros/cons of this patch?
>>
>> Pros: PVH guests with 4GB of memory or more will work. :-)
> 
> They never worked before? Or is it a regression? If it is a regression
> when did it appear?

Hmm, seems we are lucky: Linux kernel will not try to map any memory
there (I just tested it). So we don't need that patch in 4.10 for Linux
running as PVH guest. Not sure about BSD, though.


Juergen

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

Re: [Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH

[Juergen Gross]

On 05/12/17 16:23, Julien Grall wrote:
> Hi Juergen,
> 
> On 04/12/17 15:49, Juergen Gross wrote:
>> On 21/11/17 12:06, Juergen Gross wrote:
>>> The "special pages" for PVH guests include the frames for console and
>>> Xenstore ring buffers. Those have to be marked as "Reserved" in the
>>> guest's E820 map, as otherwise conflicts might arise later e.g. when
>>> hotplugging memory into the guest.
>>>
>>> Signed-off-by: Juergen Gross 
>>> ---
>>> This is a bugfix for PVH guests. Please consider for 4.10.
>>
>> Ping?
> 
> I was waiting an ack from tools maintainers before looking for a release
> perspective.
> 
> I would recommend to tag your patch is 4.10 to help reviewers prioritize
> review on your patch. I have done it now.
> 
> I am looking at releasing Xen 4.10 in the next few days. Can you explain
> the pros/cons of this patch?

Pros: PVH guests with 4GB of memory or more will work. :-)
Cons: There is a more general solution (as Roger pointed out), but
  this would require much more work.


Juergen

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

[Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH

[Ian Jackson]

Julien Grall writes ("[for-4.10] Re: [Xen-devel] [PATCH v2] tools/libxl: mark 
special pages as reserved in e820 map for PVH"):
> I would recommend to tag your patch is 4.10 to help reviewers prioritize 
> review on your patch. I have done it now.

Thanks.  Looking at the thread, I would have liked to see an answer to
this comment by Roger:

| Albeit I would also prefer this to not be PVH specific. Ideally I
| would like both PVH and HVM to share the logic to mark the reserved
| regions in the memory map. I guess this can be fixed afterwards by
| moving away this logic from hvmloader and handling the creation of
| the memory map for both HVM and PVH in libxl.

But it seems to be a bugfix and has had review from the x86
perspective, so:

Acked-by: Ian Jackson 


If anyone is feeling up to doing some improvement, I would like to see
a rework of the algorithm to avoid this error-prone duplicated-
information construction:

  +/* Add mmio entry for PVH. */
  +if (dom->mmio_size && d_config->b_info.type == LIBXL_DOMAIN_TYPE_PVH)
  +e820_entries++;

  @@ -564,6 +567,14 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,

  +/* mmio area */
  +if (dom->mmio_size && d_config->b_info.type == LIBXL_DOMAIN_TYPE_PVH) {
  +e820[nr].addr = dom->mmio_start;
  +e820[nr].size = dom->mmio_size;
  +e820[nr].type = E820_RESERVED;
  +nr++;
  +}
  +

That is, there should be no separate pre-calculation of the number of
entries.  There would have to be an expanding array instead.

Regards,
Ian.

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

[Xen-devel] [for-4.10] Re: [PATCH v2] tools/libxl: mark special pages as reserved in e820 map for PVH

[Julien Grall]

Hi Juergen,

On 04/12/17 15:49, Juergen Gross wrote:

On 21/11/17 12:06, Juergen Gross wrote:

The "special pages" for PVH guests include the frames for console and
Xenstore ring buffers. Those have to be marked as "Reserved" in the
guest's E820 map, as otherwise conflicts might arise later e.g. when
hotplugging memory into the guest.

Signed-off-by: Juergen Gross 
---
This is a bugfix for PVH guests. Please consider for 4.10.


Ping?


I was waiting an ack from tools maintainers before looking for a release
perspective.

I would recommend to tag your patch is 4.10 to help reviewers prioritize 
review on your patch. I have done it now.


I am looking at releasing Xen 4.10 in the next few days. Can you explain 
the pros/cons of this patch?


Cheers,




---
  tools/libxl/libxl_x86.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 5f91fe4f92..d82013f6ed 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -530,6 +530,9 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,
  if (d_config->rdms[i].policy != LIBXL_RDM_RESERVE_POLICY_INVALID)
  e820_entries++;
  
+/* Add mmio entry for PVH. */

+if (dom->mmio_size && d_config->b_info.type == LIBXL_DOMAIN_TYPE_PVH)
+e820_entries++;
  
  /* If we should have a highmem range. */

  if (highmem_size)
@@ -564,6 +567,14 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,
  nr++;
  }
  
+/* mmio area */

+if (dom->mmio_size && d_config->b_info.type == LIBXL_DOMAIN_TYPE_PVH) {
+e820[nr].addr = dom->mmio_start;
+e820[nr].size = dom->mmio_size;
+e820[nr].type = E820_RESERVED;
+nr++;
+}
+
  for (i = 0; i < MAX_ACPI_MODULES; i++) {
  if (dom->acpi_modules[i].length) {
  e820[nr].addr = dom->acpi_modules[i].guest_addr_out & ~(page_size 
- 1);




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



--
Julien Grall

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