* Re: [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size
2011-07-26 22:08 [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size Donald Dutile
@ 2011-07-26 22:32 ` Alex Williamson
2011-07-27 7:48 ` Michael S. Tsirkin
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Alex Williamson @ 2011-07-26 22:32 UTC (permalink / raw)
To: Donald Dutile; +Cc: kvm, mst
On Tue, 2011-07-26 at 18:08 -0400, Donald Dutile wrote:
> The bcm5761 provides a PCIe Cap structure (capid=0x10)
> that is invalid, providing one that is 8 bytes shorter
> than the v2 PCIe spec defines.
> This leads to a memory corruption when mapped for device-assigment.
>
> Add a check in assigned_device_pci_cap_init() to correct
> this hw error for this device, and try to catch other ones
> and print warnings if they exists.
>
> Signed-off-by: Donald Dutile <ddutile@redhat.com>
> cc: Alex Williamson <alex.williamson@redhat.com>
> cc: Michael S. Tsirking <mst@redhat.com>
> ---
Acked-by: Alex Williamson <alex.williamson@redhat.com>
> hw/device-assignment.c | 29 ++++++++++++++++++++++++-----
> 1 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 36ad6b0..e073840 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1419,18 +1419,37 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
> }
>
> if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_EXP, 0))) {
> - uint8_t version;
> + uint8_t version, size;
> uint16_t type, devctl, lnkcap, lnksta;
> uint32_t devcap;
> - int size = 0x3c; /* version 2 size */
>
> version = pci_get_byte(pci_dev->config + pos + PCI_EXP_FLAGS);
> version &= PCI_EXP_FLAGS_VERS;
> if (version == 1) {
> size = 0x14;
> - } else if (version > 2) {
> - fprintf(stderr, "Unsupported PCI express capability version %d\n",
> - version);
> + } else if (version == 2) {
> + /*
> + * Check for non-std size, accept reduced size to 0x34,
> + * which is what bcm5761 implemented, violating the
> + * PCIe v3.0 spec that regs should exist and be read as 0,
> + * not optionally provided and shorten the struct size.
> + */
> + size = MIN(0x3c, PCI_CONFIG_SPACE_SIZE - pos);
> + if (size < 0x34) {
> + fprintf(stderr,
> + "%s: Invalid size PCIe cap-id 0x%x \n",
> + __func__, PCI_CAP_ID_EXP);
> + return -EINVAL;
> + } else if (size != 0x3c) {
> + fprintf(stderr,
> + "WARNING, %s: PCIe cap-id 0x%x has "
> + "non-standard size 0x%x; std size should be 0x3c \n",
> + __func__, PCI_CAP_ID_EXP, size);
> + }
> + } else {
> + fprintf(stderr,
> + "%s: Unsupported PCI express capability version %d\n",
> + __func__, version);
> return -EINVAL;
> }
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size
2011-07-26 22:08 [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size Donald Dutile
2011-07-26 22:32 ` Alex Williamson
@ 2011-07-27 7:48 ` Michael S. Tsirkin
2011-07-27 9:27 ` Avi Kivity
2011-08-01 15:50 ` Marcelo Tosatti
3 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2011-07-27 7:48 UTC (permalink / raw)
To: Donald Dutile; +Cc: kvm, alex.williamson
On Tue, Jul 26, 2011 at 06:08:09PM -0400, Donald Dutile wrote:
> The bcm5761 provides a PCIe Cap structure (capid=0x10)
> that is invalid, providing one that is 8 bytes shorter
> than the v2 PCIe spec defines.
> This leads to a memory corruption when mapped for device-assigment.
>
> Add a check in assigned_device_pci_cap_init() to correct
> this hw error for this device, and try to catch other ones
> and print warnings if they exists.
>
> Signed-off-by: Donald Dutile <ddutile@redhat.com>
> cc: Alex Williamson <alex.williamson@redhat.com>
> cc: Michael S. Tsirking <mst@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> hw/device-assignment.c | 29 ++++++++++++++++++++++++-----
> 1 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 36ad6b0..e073840 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1419,18 +1419,37 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
> }
>
> if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_EXP, 0))) {
> - uint8_t version;
> + uint8_t version, size;
> uint16_t type, devctl, lnkcap, lnksta;
> uint32_t devcap;
> - int size = 0x3c; /* version 2 size */
>
> version = pci_get_byte(pci_dev->config + pos + PCI_EXP_FLAGS);
> version &= PCI_EXP_FLAGS_VERS;
> if (version == 1) {
> size = 0x14;
> - } else if (version > 2) {
> - fprintf(stderr, "Unsupported PCI express capability version %d\n",
> - version);
> + } else if (version == 2) {
> + /*
> + * Check for non-std size, accept reduced size to 0x34,
> + * which is what bcm5761 implemented, violating the
> + * PCIe v3.0 spec that regs should exist and be read as 0,
> + * not optionally provided and shorten the struct size.
> + */
> + size = MIN(0x3c, PCI_CONFIG_SPACE_SIZE - pos);
> + if (size < 0x34) {
> + fprintf(stderr,
> + "%s: Invalid size PCIe cap-id 0x%x \n",
> + __func__, PCI_CAP_ID_EXP);
> + return -EINVAL;
> + } else if (size != 0x3c) {
> + fprintf(stderr,
> + "WARNING, %s: PCIe cap-id 0x%x has "
> + "non-standard size 0x%x; std size should be 0x3c \n",
> + __func__, PCI_CAP_ID_EXP, size);
> + }
> + } else {
> + fprintf(stderr,
> + "%s: Unsupported PCI express capability version %d\n",
> + __func__, version);
> return -EINVAL;
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size
2011-07-26 22:08 [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size Donald Dutile
2011-07-26 22:32 ` Alex Williamson
2011-07-27 7:48 ` Michael S. Tsirkin
@ 2011-07-27 9:27 ` Avi Kivity
2011-08-01 15:50 ` Marcelo Tosatti
3 siblings, 0 replies; 8+ messages in thread
From: Avi Kivity @ 2011-07-27 9:27 UTC (permalink / raw)
To: Donald Dutile; +Cc: kvm, alex.williamson, mst
On 07/27/2011 01:08 AM, Donald Dutile wrote:
> The bcm5761 provides a PCIe Cap structure (capid=0x10)
> that is invalid, providing one that is 8 bytes shorter
> than the v2 PCIe spec defines.
> This leads to a memory corruption when mapped for device-assigment.
>
> Add a check in assigned_device_pci_cap_init() to correct
> this hw error for this device, and try to catch other ones
> and print warnings if they exists.
Applied, thanks.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size
2011-07-26 22:08 [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size Donald Dutile
` (2 preceding siblings ...)
2011-07-27 9:27 ` Avi Kivity
@ 2011-08-01 15:50 ` Marcelo Tosatti
2011-08-01 15:53 ` Avi Kivity
3 siblings, 1 reply; 8+ messages in thread
From: Marcelo Tosatti @ 2011-08-01 15:50 UTC (permalink / raw)
To: Donald Dutile; +Cc: kvm, alex.williamson, mst
On Tue, Jul 26, 2011 at 06:08:09PM -0400, Donald Dutile wrote:
> The bcm5761 provides a PCIe Cap structure (capid=0x10)
> that is invalid, providing one that is 8 bytes shorter
> than the v2 PCIe spec defines.
> This leads to a memory corruption when mapped for device-assigment.
>
> Add a check in assigned_device_pci_cap_init() to correct
> this hw error for this device, and try to catch other ones
> and print warnings if they exists.
>
> Signed-off-by: Donald Dutile <ddutile@redhat.com>
> cc: Alex Williamson <alex.williamson@redhat.com>
> cc: Michael S. Tsirking <mst@redhat.com>
Patch does not apply cleanly, complaints about trailing whitespaces.
Please regenerate against current git tree, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size
2011-08-01 15:50 ` Marcelo Tosatti
@ 2011-08-01 15:53 ` Avi Kivity
2011-08-01 15:56 ` Marcelo Tosatti
0 siblings, 1 reply; 8+ messages in thread
From: Avi Kivity @ 2011-08-01 15:53 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Donald Dutile, kvm, alex.williamson, mst
On 08/01/2011 06:50 PM, Marcelo Tosatti wrote:
> On Tue, Jul 26, 2011 at 06:08:09PM -0400, Donald Dutile wrote:
> > The bcm5761 provides a PCIe Cap structure (capid=0x10)
> > that is invalid, providing one that is 8 bytes shorter
> > than the v2 PCIe spec defines.
> > This leads to a memory corruption when mapped for device-assigment.
> >
> > Add a check in assigned_device_pci_cap_init() to correct
> > this hw error for this device, and try to catch other ones
> > and print warnings if they exists.
> >
> > Signed-off-by: Donald Dutile<ddutile@redhat.com>
> > cc: Alex Williamson<alex.williamson@redhat.com>
> > cc: Michael S. Tsirking<mst@redhat.com>
>
> Patch does not apply cleanly, complaints about trailing whitespaces.
>
> Please regenerate against current git tree, thanks.
>
I thought I applied it already, I even remember the trailing whitespace
complaints (which git fixed for me).
I hope we didn't lose other patches this way.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size
2011-08-01 15:53 ` Avi Kivity
@ 2011-08-01 15:56 ` Marcelo Tosatti
2011-08-01 16:37 ` Avi Kivity
0 siblings, 1 reply; 8+ messages in thread
From: Marcelo Tosatti @ 2011-08-01 15:56 UTC (permalink / raw)
To: Avi Kivity; +Cc: Donald Dutile, kvm, alex.williamson, mst
On Mon, Aug 01, 2011 at 06:53:49PM +0300, Avi Kivity wrote:
> On 08/01/2011 06:50 PM, Marcelo Tosatti wrote:
> >On Tue, Jul 26, 2011 at 06:08:09PM -0400, Donald Dutile wrote:
> >> The bcm5761 provides a PCIe Cap structure (capid=0x10)
> >> that is invalid, providing one that is 8 bytes shorter
> >> than the v2 PCIe spec defines.
> >> This leads to a memory corruption when mapped for device-assigment.
> >>
> >> Add a check in assigned_device_pci_cap_init() to correct
> >> this hw error for this device, and try to catch other ones
> >> and print warnings if they exists.
> >>
> >> Signed-off-by: Donald Dutile<ddutile@redhat.com>
> >> cc: Alex Williamson<alex.williamson@redhat.com>
> >> cc: Michael S. Tsirking<mst@redhat.com>
> >
> >Patch does not apply cleanly, complaints about trailing whitespaces.
> >
> >Please regenerate against current git tree, thanks.
> >
>
> I thought I applied it already, I even remember the trailing
> whitespace complaints (which git fixed for me).
>
> I hope we didn't lose other patches this way.
OK. Its not lost:
commit f9c29774d2174df6ffc20becec20928948198914
Author: Donald Dutile <ddutile@redhat.com>
Date: Tue Jul 26 18:08:09 2011 -0400
device-assignment: handle device with incorrect PCIe Cap structure size
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] dev-assignment: handle device with incorrect PCIe Cap structure size
2011-08-01 15:56 ` Marcelo Tosatti
@ 2011-08-01 16:37 ` Avi Kivity
0 siblings, 0 replies; 8+ messages in thread
From: Avi Kivity @ 2011-08-01 16:37 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Donald Dutile, kvm, alex.williamson, mst
On 08/01/2011 06:56 PM, Marcelo Tosatti wrote:
> OK. Its not lost:
>
> commit f9c29774d2174df6ffc20becec20928948198914
> Author: Donald Dutile<ddutile@redhat.com>
> Date: Tue Jul 26 18:08:09 2011 -0400
>
> device-assignment: handle device with incorrect PCIe Cap structure size
>
Ah, I looked for it in the kernel tree instead of qemu.
I look silly either way, but at least the patch is in.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 8+ messages in thread