On Sun, Jul 24, 2011 at 11:12:44AM +0300, Michael S. Tsirkin wrote:
> On Fri, Jul 22, 2011 at 02:35:47PM -0700, Chris Wright wrote:
> > * Alex Williamson (alex.williamson@xxxxxxxxxx) wrote:
> > > On Fri, 2011-07-22 at 14:24 -0700, Chris Wright wrote:
> > > > * Donald Dutile (ddutile@xxxxxxxxxx) wrote:
> > > > > diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> > > > > index 36ad6b0..34db52e 100644
> > > > > --- a/hw/device-assignment.c
> > > > > +++ b/hw/device-assignment.c
> > > > > @@ -1419,16 +1419,18 @@ 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) {
> > > > > + } else if (version == 2) {
> > > > > + /* don't include slot cap/stat/ctrl 2 regs; only support endpoints */
> > > > > + size = 0x34;
> > > >
> > > > That doesn't look correct to me. The size is fixed, just that some
> > > > registers are Reserved Zero when they do not apply (e.g. endpoint only).
> > >
> > > Apparently it can be interpreted differently. In this case, we've seen
> > > a tg3 device expose a v2 PCI express capability at offset 0xcc. Using
> > > 0x3c bytes, we extend 8 bytes past the legacy config space area :(
> >
> > Wow, that device sounds broken to me. The spec is pretty clear.
>
> Yes, I agree it's broken. Looks like something that
> happens when a device is designed in parallel with the spec.
>
> What bothers me is this patch seems to make devices that do behave
> correctly out of spec (registers will be writeable by default) -
> correct?
>
> How about we check for overflow and only do the hacks
> if it happens?
>
> Also, the code to initialize slot and root control registers is still
> there: it would seem that running it will corrupt memmory beyond the
> config array?
I take this last bit back: registers we touch are at offset < 0x34.
Sorry about the noise. But the question about read-only registers
still stands.
>
> --
> MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
