[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more
From: |
Wu, Feng |
Subject: |
Re: [Qemu-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size |
Date: |
Thu, 6 Aug 2015 13:27:39 +0000 |
> -----Original Message-----
> From: Stefano Stabellini [mailto:address@hidden
> Sent: Thursday, August 06, 2015 6:43 PM
> To: Wu, Feng
> Cc: address@hidden; address@hidden;
> address@hidden
> 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 <address@hidden>
> > ---
> > 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