Re: [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@xxxxxxxxxxxxx]
> Sent: Thursday, August 06, 2015 6:43 PM
> To: Wu, Feng
> Cc: stefano.stabellini@xxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxx;
> qemu-devel@xxxxxxxxxx
> Subject: Re: [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G
> size
> 
> On Wed, 5 Aug 2015, Feng Wu wrote:
> > This patch corrects a logic error when handling 64-bt bar with
> > more than 4G size.
> >
> > With 64-bit Bar, it has two items in PCIDevice: io_regions[x]
> > and io_regions[x+1], io_regions[x] has all the informations for
> > this BAR, while io_regions[x+1] contains nothing, so we need to
> > get the size from io_regions[x] when handling XEN_PT_BAR_FLAG_UPPER.
> 
> That's because of the way io_regions are populated by
> xen_host_pci_get_resource, right?

Yes, xen_host_pci_get_resouce() is the source of the BAR information.

Thanks,
Feng

> 
> 
> > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > ---
> > I cannot test this patch sicne I don't have such a device, if
> > someone have it, it would be highly appreicated if he can help
> > to verfiy this patch.
> 
> I would very much appreciate if somebody could properly validate this
> patch with the right device before I actually commit it
> 
> 
> >  hw/xen/xen_pt_config_init.c |   22 +++-------------------
> >  1 files changed, 3 insertions(+), 19 deletions(-)
> >
> > diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> > index dd37be3..6fcef66 100644
> > --- a/hw/xen/xen_pt_config_init.c
> > +++ b/hw/xen/xen_pt_config_init.c
> > @@ -326,23 +326,6 @@ static int
> xen_pt_cmd_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
> >  #define XEN_PT_BAR_IO_RO_MASK     0x00000003  /* BAR ReadOnly
> mask(I/O) */
> >  #define XEN_PT_BAR_IO_EMU_MASK    0xFFFFFFFC  /* BAR emul
> mask(I/O) */
> >
> > -static bool is_64bit_bar(PCIIORegion *r)
> > -{
> > -    return !!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64);
> > -}
> > -
> > -static uint64_t xen_pt_get_bar_size(PCIIORegion *r)
> > -{
> > -    if (is_64bit_bar(r)) {
> > -        uint64_t size64;
> > -        size64 = (r + 1)->size;
> > -        size64 <<= 32;
> > -        size64 += r->size;
> > -        return size64;
> > -    }
> > -    return r->size;
> > -}
> > -
> >  static XenPTBarFlag xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
> >                                           int index)
> >  {
> > @@ -365,7 +348,7 @@ static XenPTBarFlag
> xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
> >
> >      /* check unused BAR */
> >      r = &d->io_regions[index];
> > -    if (!xen_pt_get_bar_size(r)) {
> > +    if (r->size == 0) {
> >          return XEN_PT_BAR_FLAG_UNUSED;
> >      }
> > @@ -491,8 +474,9 @@ static int
> xen_pt_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *cfg_entry,
> >          bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1);
> >          break;
> >      case XEN_PT_BAR_FLAG_UPPER:
> > +        r = &d->io_regions[index-1];
> >          bar_emu_mask = XEN_PT_BAR_ALLF;
> > -        bar_ro_mask = r_size ? r_size - 1 : 0;
> > +        bar_ro_mask = (r->size - 1) >> 32;
> >          break;
> >      default:
> >          break;
> 
> The changes look correct

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