Xen devel - Oct 2012 - [PATCH] hvm: handle PoD and grant pages in HVMOP_get_mem_type

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1350655745 -7200
# Node ID 8ebe7b80f02900d5a83e023c2833de26b70f3ff1
# Parent  3fa2ab30bb05297f30d9a33b30f29db960900971
hvm: handle PoD and grant pages in HVMOP_get_mem_type

During kexec in a ballooned PVonHVM guest the new kernel needs to check
each pfn if its backed by a mfn to find ballooned pages. Currently all
PoD and grant pages will appear as HVMMEM_mmio_dm, so the new kernel has
to assume they are ballooned. This is wrong: PoD pages may turn into
real RAM at runtime, grant pages are also RAM.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r 3fa2ab30bb05 -r 8ebe7b80f029 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4086,6 +4086,10 @@ long do_hvm_op(unsigned long op, XEN_GUE
                 a.mem_type =  HVMMEM_ram_ro;
             else if ( p2m_is_ram(t) )
                 a.mem_type =  HVMMEM_ram_rw;
+            else if ( p2m_is_magic(t) )
+                a.mem_type =  HVMMEM_ram_rw;
+            else if ( p2m_is_grant(t) )
+                a.mem_type =  HVMMEM_ram_rw;
             else
                 a.mem_type =  HVMMEM_mmio_dm;
             rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;

On Fri, Oct 19, Olaf Hering wrote:
> +            else if ( p2m_is_grant(t) )
I''m not sure if p2m_is_grant() can be true in a HVM guest.

Olaf

> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1350655745 -7200
> # Node ID 8ebe7b80f02900d5a83e023c2833de26b70f3ff1
> # Parent  3fa2ab30bb05297f30d9a33b30f29db960900971
> hvm: handle PoD and grant pages in HVMOP_get_mem_type
> 
> During kexec in a ballooned PVonHVM guest the new kernel needs to check
> each pfn if its backed by a mfn to find ballooned pages. Currently all
> PoD and grant pages will appear as HVMMEM_mmio_dm, so the new kernel has
> to assume they are ballooned. This is wrong: PoD pages may turn into
> real RAM at runtime, grant pages are also RAM.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r 3fa2ab30bb05 -r 8ebe7b80f029 xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4086,6 +4086,10 @@ long do_hvm_op(unsigned long op, XEN_GUE
>                 a.mem_type =  HVMMEM_ram_ro;
>             else if ( p2m_is_ram(t) )
>                 a.mem_type =  HVMMEM_ram_rw;
> +            else if ( p2m_is_magic(t) )
> +                a.mem_type =  HVMMEM_ram_rw;
p2m_is_magic is this bizarre thing that should just be p2m_is_pod. Can you take
advantage of this opportunity and fix it?
> +            else if ( p2m_is_grant(t) )
> +                a.mem_type =  HVMMEM_ram_rw;
Yes there can be p2m_is_grant pages in an HVM, if it is running a backend. Note
that you need to discriminate whether the grant is mapped writable in order to
return ram_rw or ram_ro.

It might be a good idea to extend this interface to return HVMMEM_grant_rw/ro.
These are, in essence, different types of ram from an HVM point of view.
However, it might be overkill for the current users.

Thanks
Andres
>             else
>                 a.mem_type =  HVMMEM_mmio_dm;
>             rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
>

On Fri, Oct 19, Andres Lagar-Cavilla wrote:
> > +            else if ( p2m_is_magic(t) )
> > +                a.mem_type =  HVMMEM_ram_rw;
> p2m_is_magic is this bizarre thing that should just be p2m_is_pod. Can
> you take advantage of this opportunity and fix it?
I will prepare a separate patch for that rename.
> > +            else if ( p2m_is_grant(t) )
> > +                a.mem_type =  HVMMEM_ram_rw;
> 
> Yes there can be p2m_is_grant pages in an HVM, if it is running a
> backend. Note that you need to discriminate whether the grant is
> mapped writable in order to return ram_rw or ram_ro.
I will put p2m_grant_map_rw into HVMMEM_ram_rw, and p2m_grant_map_ro
into HVMMEM_ram_ro.
> It might be a good idea to extend this interface to return
> HVMMEM_grant_rw/ro. These are, in essence, different types of ram from
> an HVM point of view. However, it might be overkill for the current
> users.
Would the guest really care about the info that a given page is a grant
page? What would it do with that info?


Olaf

On Mon, Oct 22, Olaf Hering wrote:
> On Fri, Oct 19, Andres Lagar-Cavilla wrote:
> > > +            else if ( p2m_is_grant(t) )
> > > +                a.mem_type =  HVMMEM_ram_rw;
> > 
> > Yes there can be p2m_is_grant pages in an HVM, if it is running a
> > backend. Note that you need to discriminate whether the grant is
> > mapped writable in order to return ram_rw or ram_ro.
> 
> I will put p2m_grant_map_rw into HVMMEM_ram_rw, and p2m_grant_map_ro
> into HVMMEM_ram_ro.
Reading through the p2m_is_* defines, p2m_grant_map_ro maps already to
HVMMEM_ram_ro. So the patch covers all cases already.

Olaf

At 16:09 +0200 on 19 Oct (1350662980), Olaf Hering
wrote:
> hvm: handle PoD and grant pages in HVMOP_get_mem_type
> 
> During kexec in a ballooned PVonHVM guest the new kernel needs to check
> each pfn if its backed by a mfn to find ballooned pages. Currently all
> PoD and grant pages will appear as HVMMEM_mmio_dm, so the new kernel has
> to assume they are ballooned. This is wrong: PoD pages may turn into
> real RAM at runtime, grant pages are also RAM.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
Applied, thanks.

Tim.

On Mon, Oct 22, Tim Deegan wrote:
> At 16:09 +0200 on 19 Oct (1350662980), Olaf Hering wrote:
> > hvm: handle PoD and grant pages in HVMOP_get_mem_type
> > 
> > During kexec in a ballooned PVonHVM guest the new kernel needs to
check
> > each pfn if its backed by a mfn to find ballooned pages. Currently all
> > PoD and grant pages will appear as HVMMEM_mmio_dm, so the new kernel
has
> > to assume they are ballooned. This is wrong: PoD pages may turn into
> > real RAM at runtime, grant pages are also RAM.
> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> Applied, thanks.
Would you backport this to 4.2 and 4.1?
Without this change the guest will get a wrong picture.
Too bad I did not consider this initially in 23298:26413986e6e0.

Olaf

At 17:33 +0200 on 22 Oct (1350927213), Olaf Hering
wrote:
> On Mon, Oct 22, Tim Deegan wrote:
> 
> > At 16:09 +0200 on 19 Oct (1350662980), Olaf Hering wrote:
> > > hvm: handle PoD and grant pages in HVMOP_get_mem_type
> > > 
> > > During kexec in a ballooned PVonHVM guest the new kernel needs to
check
> > > each pfn if its backed by a mfn to find ballooned pages.
Currently all
> > > PoD and grant pages will appear as HVMMEM_mmio_dm, so the new
kernel has
> > > to assume they are ballooned. This is wrong: PoD pages may turn
into
> > > real RAM at runtime, grant pages are also RAM.
> > > 
> > > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > 
> > Applied, thanks.
> 
> Would you backport this to 4.2 and 4.1?
Actually, I don''t handle backports.  IIRC Jan owns 4.2-testing and Keir
4.1-testing; I''ve CC''d them both.

Cheers,

Tim.
> Without this change the guest will get a wrong picture.
> Too bad I did not consider this initially in 23298:26413986e6e0.
> 
> Olaf
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

>>> On 22.10.12 at 18:13, Tim Deegan <tim@xen.org> wrote:
> At 17:33 +0200 on 22 Oct (1350927213), Olaf Hering wrote:
>> On Mon, Oct 22, Tim Deegan wrote:
>> 
>> > At 16:09 +0200 on 19 Oct (1350662980), Olaf Hering wrote:
>> > > hvm: handle PoD and grant pages in HVMOP_get_mem_type
>> > > 
>> > > During kexec in a ballooned PVonHVM guest the new kernel
needs to check
>> > > each pfn if its backed by a mfn to find ballooned pages.
Currently all
>> > > PoD and grant pages will appear as HVMMEM_mmio_dm, so the new
kernel has
>> > > to assume they are ballooned. This is wrong: PoD pages may
turn into
>> > > real RAM at runtime, grant pages are also RAM.
>> > > 
>> > > Signed-off-by: Olaf Hering <olaf@aepfle.de>
>> > 
>> > Applied, thanks.
>> 
>> Would you backport this to 4.2 and 4.1?
> 
> Actually, I don''t handle backports.  IIRC Jan owns 4.2-testing and
Keir
> 4.1-testing; I''ve CC''d them both.
Actually, I handle both and will take care of it within the next few
days (hopefully before the end of the week).

Jan