On Thu, Feb 07, 2013 at 02:33:28PM +0100, Laszlo Ersek wrote: > Apologies in advance for asking a possibly exorbitantly lame question... > > On 02/06/13 10:47, Vadim Rozenfeld wrote: > > On Tue, 2013-02-05 at 13:58 +0200, Michael S. Tsirkin wrote: > >> On Tue, Feb 05, 2013 at 03:45:38PM +0400, Michael Tokarev wrote: > >>> 05.02.2013 15:31, Vadim Rozenfeld wrote: > >>>> On Mon, 2013-02-04 at 08:41 -0600, Anthony Liguori wrote: > > >>>>> But adding feature bits an altering the config size doesn't constitute a > >>>>> change in the software interface IMHO. > >>>> > >>>> I agree, feature bits are good. The only one problem with them, is that > >>>> driver usually doesn't have access to PCI space during the driver > >>>> loading phase. > >>> > >>> Vadim, can you please describe in a bit more details what the actual issue > >>> here is, from the windows or windows driver point of view? > > > If a driver receives descriptor with memory type resources it needs to > > map physical address to logical. This process requires the number of > > bytes to be mapped, and it is the best place for sanity check to verify > > that the resources are valid and not corrupted. We usually do it by > > comparing the resource size with some predefined value, that we expect > > to see. > > >>> Is it really > >>> that bad that the config space size changed? Why it has this effect? > > > Because in this case it's hard to distinguish between resource's > > corruption and HW update. > > In theory, would the following approach work? > > (1) insist on revision-id being 0, > > (2) only require a minimum size of VIRTIO_PCI_CONFIG_NOMSI == 20 bytes > from the iomem BAR (= common virtio header, guaranteed by the virtio > spec for revision-id==0) when loading the driver, > > (3) at first, only map this initial (guaranteed) part of the resource, > > (4) this should provide access to the feature bits, allowing > verification of the full BAR size, including MSI-X fields and device > type specific fields, > > (5a) remap the BAR with full size if there's a match, > (5b) if there's a mismatch, set "permanent hardware failure" or some > such in the loaded drier. > > It would be probably overkill; I'm just wondering if it made sense in > theory. > > Thanks > Laszlo
Linux simply maps the whole BAR. It's an IO BAR so never bigger than 256 bytes. It then checks feature bits before access. We could also range-check versus the BAR size, though we don't at the moment. The API looks like this: if (virtio_config_val(vi->vdev, VIRTIO_NET_F_STATUS, offsetof(struct virtio_net_config, status), &v) < 0) -- MST