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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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?


> 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