* [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
@ 2015-08-05 2:02 Feng Wu
2015-08-06 10:43 ` Stefano Stabellini
2015-08-12 6:58 ` [Xen-devel] " Jan Beulich
0 siblings, 2 replies; 8+ messages in thread
From: Feng Wu @ 2015-08-05 2:02 UTC (permalink / raw)
To: stefano.stabellini; +Cc: xen-devel, Feng Wu, qemu-devel
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.
Signed-off-by: Feng Wu <feng.wu@intel.com>
---
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.
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;
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
2015-08-05 2:02 [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size Feng Wu
@ 2015-08-06 10:43 ` Stefano Stabellini
2015-08-06 13:27 ` Wu, Feng
2015-08-12 6:58 ` [Xen-devel] " Jan Beulich
1 sibling, 1 reply; 8+ messages in thread
From: Stefano Stabellini @ 2015-08-06 10:43 UTC (permalink / raw)
To: Feng Wu; +Cc: xen-devel, qemu-devel, stefano.stabellini
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@intel.com>
> ---
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
2015-08-06 10:43 ` Stefano Stabellini
@ 2015-08-06 13:27 ` Wu, Feng
0 siblings, 0 replies; 8+ messages in thread
From: Wu, Feng @ 2015-08-06 13:27 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, Wu, Feng, qemu-devel
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: Thursday, August 06, 2015 6:43 PM
> To: Wu, Feng
> Cc: stefano.stabellini@eu.citrix.com; xen-devel@lists.xensource.com;
> qemu-devel@nongnu.org
> 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@intel.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
2015-08-05 2:02 [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size Feng Wu
2015-08-06 10:43 ` Stefano Stabellini
@ 2015-08-12 6:58 ` Jan Beulich
2015-08-12 7:10 ` Wu, Feng
1 sibling, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2015-08-12 6:58 UTC (permalink / raw)
To: Feng Wu; +Cc: xen-devel, qemu-devel, stefano.stabellini
>>> On 05.08.15 at 04:02, <feng.wu@intel.com> wrote:
> @@ -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];
Perhaps worth an assert(index > 0)?
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
2015-08-12 6:58 ` [Xen-devel] " Jan Beulich
@ 2015-08-12 7:10 ` Wu, Feng
2015-08-12 8:43 ` Jan Beulich
0 siblings, 1 reply; 8+ messages in thread
From: Wu, Feng @ 2015-08-12 7:10 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Wu, Feng, qemu-devel, stefano.stabellini
> -----Original Message-----
> From: qemu-devel-bounces+feng.wu=intel.com@nongnu.org
> [mailto:qemu-devel-bounces+feng.wu=intel.com@nongnu.org] On Behalf Of
> Jan Beulich
> Sent: Wednesday, August 12, 2015 2:59 PM
> To: Wu, Feng
> Cc: xen-devel@lists.xensource.com; qemu-devel@nongnu.org;
> stefano.stabellini@eu.citrix.com
> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit
> bar with more than 4G size
>
> >>> On 05.08.15 at 04:02, <feng.wu@intel.com> wrote:
> > @@ -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];
>
> Perhaps worth an assert(index > 0)?
No problem, I will add it. BTW, do you have any other comments about this patch? If no, I am
going to send out the new version with this changes.
Thanks,
Feng
>
> Jan
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
2015-08-12 7:10 ` Wu, Feng
@ 2015-08-12 8:43 ` Jan Beulich
2015-08-12 8:53 ` Wu, Feng
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2015-08-12 8:43 UTC (permalink / raw)
To: Feng Wu; +Cc: xen-devel, qemu-devel, stefano.stabellini
>>> On 12.08.15 at 09:10, <feng.wu@intel.com> wrote:
>
>> -----Original Message-----
>> From: qemu-devel-bounces+feng.wu=intel.com@nongnu.org
>> [mailto:qemu-devel-bounces+feng.wu=intel.com@nongnu.org] On Behalf Of
>> Jan Beulich
>> Sent: Wednesday, August 12, 2015 2:59 PM
>> To: Wu, Feng
>> Cc: xen-devel@lists.xensource.com; qemu-devel@nongnu.org;
>> stefano.stabellini@eu.citrix.com
>> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit
>> bar with more than 4G size
>>
>> >>> On 05.08.15 at 04:02, <feng.wu@intel.com> wrote:
>> > @@ -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];
>>
>> Perhaps worth an assert(index > 0)?
>
> No problem, I will add it. BTW, do you have any other comments about this
> patch? If no, I am
> going to send out the new version with this changes.
No - everything else looks to make sense (but continues to need
testing).
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
2015-08-12 8:43 ` Jan Beulich
@ 2015-08-12 8:53 ` Wu, Feng
2015-08-12 14:29 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 8+ messages in thread
From: Wu, Feng @ 2015-08-12 8:53 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Wu, Feng, qemu-devel, stefano.stabellini
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Wednesday, August 12, 2015 4:43 PM
> To: Wu, Feng
> Cc: stefano.stabellini@eu.citrix.com; xen-devel@lists.xensource.com;
> qemu-devel@nongnu.org
> Subject: RE: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit
> bar with more than 4G size
>
> >>> On 12.08.15 at 09:10, <feng.wu@intel.com> wrote:
>
> >
> >> -----Original Message-----
> >> From: qemu-devel-bounces+feng.wu=intel.com@nongnu.org
> >> [mailto:qemu-devel-bounces+feng.wu=intel.com@nongnu.org] On Behalf Of
> >> Jan Beulich
> >> Sent: Wednesday, August 12, 2015 2:59 PM
> >> To: Wu, Feng
> >> Cc: xen-devel@lists.xensource.com; qemu-devel@nongnu.org;
> >> stefano.stabellini@eu.citrix.com
> >> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle
> 64-bit
> >> bar with more than 4G size
> >>
> >> >>> On 05.08.15 at 04:02, <feng.wu@intel.com> wrote:
> >> > @@ -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];
> >>
> >> Perhaps worth an assert(index > 0)?
> >
> > No problem, I will add it. BTW, do you have any other comments about this
> > patch? If no, I am
> > going to send out the new version with this changes.
>
> No - everything else looks to make sense (but continues to need
> testing).
>
I don't have such a device in hand. Can anybody who has such a device help to test this
patch? It would be highly appreciated!
Thanks,
Feng
> Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size
2015-08-12 8:53 ` Wu, Feng
@ 2015-08-12 14:29 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 8+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-08-12 14:29 UTC (permalink / raw)
To: Wu, Feng; +Cc: xen-devel, qemu-devel, Jan Beulich, stefano.stabellini
On Wed, Aug 12, 2015 at 08:53:44AM +0000, Wu, Feng wrote:
>
>
> > -----Original Message-----
> > From: Jan Beulich [mailto:JBeulich@suse.com]
> > Sent: Wednesday, August 12, 2015 4:43 PM
> > To: Wu, Feng
> > Cc: stefano.stabellini@eu.citrix.com; xen-devel@lists.xensource.com;
> > qemu-devel@nongnu.org
> > Subject: RE: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle 64-bit
> > bar with more than 4G size
> >
> > >>> On 12.08.15 at 09:10, <feng.wu@intel.com> wrote:
> >
> > >
> > >> -----Original Message-----
> > >> From: qemu-devel-bounces+feng.wu=intel.com@nongnu.org
> > >> [mailto:qemu-devel-bounces+feng.wu=intel.com@nongnu.org] On Behalf Of
> > >> Jan Beulich
> > >> Sent: Wednesday, August 12, 2015 2:59 PM
> > >> To: Wu, Feng
> > >> Cc: xen-devel@lists.xensource.com; qemu-devel@nongnu.org;
> > >> stefano.stabellini@eu.citrix.com
> > >> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v1] xenpt: Properly handle
> > 64-bit
> > >> bar with more than 4G size
> > >>
> > >> >>> On 05.08.15 at 04:02, <feng.wu@intel.com> wrote:
> > >> > @@ -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];
> > >>
> > >> Perhaps worth an assert(index > 0)?
> > >
> > > No problem, I will add it. BTW, do you have any other comments about this
> > > patch? If no, I am
> > > going to send out the new version with this changes.
> >
> > No - everything else looks to make sense (but continues to need
> > testing).
> >
>
> I don't have such a device in hand. Can anybody who has such a device help to test this
> patch? It would be highly appreciated!
Um, 4GB MMIO bars? Wouldn't the Nvidia Quadro K6000 12GB GDDR do it? I am sure
that Intel would be OK expensing $4K of hardware :-)
>
> Thanks,
> Feng
>
> > Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-08-12 14:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-05 2:02 [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size Feng Wu
2015-08-06 10:43 ` Stefano Stabellini
2015-08-06 13:27 ` Wu, Feng
2015-08-12 6:58 ` [Xen-devel] " Jan Beulich
2015-08-12 7:10 ` Wu, Feng
2015-08-12 8:43 ` Jan Beulich
2015-08-12 8:53 ` Wu, Feng
2015-08-12 14:29 ` Konrad Rzeszutek Wilk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).