Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git

[Michael S. Tsirkin]

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