Re: [Xen-devel] [Xen-ARM: Linux] Grant table address, xen_hvm_resume_frames, is a phys_addr not a pfn

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 3 Dec 2013, Ian Campbell wrote:
> On Mon, 2013-12-02 at 21:25 +0000, Eric Trudeau wrote:
> > While working on our embedded ARM platform, we discovered a bug in the XEN
> > Linux ARM code where the grant table is being mapped into guest physical
> > memory at the wrong address because enlighten.c treats the
> > xen_hvm_resume_frames variable as if it was a pfn when, in fact, it is a 
> > physical
> > address.  Thus, instead of the address specified in the hypervisor node of 
> > the
> > device tree, e.g. 0xB0000000, the actual guest physical address is 
> > 0x000B0000.
> > 
> > Below is a _possible_ patch a developer from the Xen Linux team could try or
> > at least start from.  The kernel that we are using is based on an
> > older version, but I have made this suggested patch on recent linux-stable 
> > code.
> > It is not tested on the latest linux-stable code, but it has been tested on 
> > linux as
> > of commit bee980d9e9642e96351fa3ca9077b853ecf62f57.
> > 
> > Eric Trudeau
> > ---
> >     XEN: Grant table address, xen_hvm_resume_frames, is a phys_addr not a 
> > pfn
> > 
> >     xen_hvm_resume_frames stores the physical address of the grant table.
> >     englighten.c was incorrectly setting it as if it was a page frame 
> > number.
> >     This caused the table to be mapped into the guest at an unexpected 
> > physical
> >     address.
> > 
> >     Additionally, a warning is improved to include the grant table address 
> > which
> >     failed in xen_remap.
> > 
> >     Signed-off-by: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
> 
> Agreed.
> 
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Right, I'll add it to xentip.


> I also think that xen_hvm_resume_frames has a confusing name and an
> incorrect type (unsigned long). The name makes it sound like a frame
> number and the type should be either a paddr_t or a resource_size_t (a
> typedef to paddr_t, not obvious to me what the distinction is). Using
> unsigned long risks truncation of large MMIO addresses on 32-bit
> kernels.

indeed


> > ---
> >  arch/arm/xen/enlighten.c  | 4 ++--
> >  drivers/xen/grant-table.c | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index 83e4f95..ceaf9d5 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -224,10 +224,10 @@ static int __init xen_guest_init(void)
> >         }
> >         if (of_address_to_resource(node, GRANT_TABLE_PHYSADDR, &res))
> >                 return 0;
> > -       xen_hvm_resume_frames = res.start >> PAGE_SHIFT;
> > +       xen_hvm_resume_frames = res.start;
> >         xen_events_irq = irq_of_parse_and_map(node, 0);
> >         pr_info("Xen %s support found, events_irq=%d 
> > gnttab_frame_pfn=%lx\n",
> > -                       version, xen_events_irq, xen_hvm_resume_frames);
> > +                version, xen_events_irq, (xen_hvm_resume_frames >> 
> > PAGE_SHIFT));
> >         xen_domain_type = XEN_HVM_DOMAIN;
> > 
> >         xen_setup_features();
> > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> > index 62ccf54..0b4a741 100644
> > --- a/drivers/xen/grant-table.c
> > +++ b/drivers/xen/grant-table.c
> > @@ -1174,7 +1174,7 @@ static int gnttab_setup(void)
> >                 gnttab_shared.addr = xen_remap(xen_hvm_resume_frames,
> >                                                 PAGE_SIZE * max_nr_gframes);
> >                 if (gnttab_shared.addr == NULL) {
> > -                       pr_warn("Failed to ioremap gnttab share frames!\n");
> > +                       pr_warn("Failed to ioremap gnttab share frames 
> > (addr=0x%08lx)!\n", xen_hvm_resume_frames);
> >                         return -ENOMEM;
> >                 }
> >         }
> > 
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel