Re: [Qemu-devel] [PATCH] virtio-pci: replace byte swap hack

[Blue Swirl]

On Mon, Jan 7, 2013 at 4:11 PM, Michael S. Tsirkin <address@hidden> wrote:
> On Sun, Jan 06, 2013 at 08:04:39PM +0000, Blue Swirl wrote:
>> On Sun, Jan 6, 2013 at 6:25 PM, Andreas Färber <address@hidden> wrote:
>> > Am 06.01.2013 14:17, schrieb Alexander Graf:
>> >>
>> >> On 30.12.2012, at 13:55, Blue Swirl wrote:
>> >>
>> >>> Remove byte swaps by declaring the config space
>> >>> as native endian.
>> >>
>> >> This is wrong. Virtio-pci config space is split into 2 regions. One with 
>> >> native endianness, the other one with little endian.
>> >
>> > Can that MemoryRegion be split in two?
>>
>> Yes, but unfortunately the offset for the second region depends on if
>> MSIX is enabled or not. PCI layer manages these bits without the
>> device seeing any changes.
>>
>> This could be handled by introducing a callback at PCI layer to inform
>> interested devices about changes to MSIX setup, or even generalized:
>> inform devices about changes within any set of bits specified by the
>> device.
>
> We already have a generic config_write callback and even use it in
> virtio pci: virtio_write_config.  So you could simply do there:
>
>         if (region size != VIRTIO_PCI_CONFIG(dev)) {
>                 resize regions
>         }
>
> We would also have to resize to the default setup on
> vm load and on vm reset.
>
> Overall not sure whether this would make the code cleaner or uglier.

I think it would be a net cleanup. Most of the ugliness comes from the
poor device architecture.

There could be (unmeasurably) small performance gains since accesses
to the two regions would be dispatched directly to the handlers. But
if the MSIX mode bit is toggled very often compared to the accesses to
config registers, it could actually cause some slow down due to
adjustment to the offset with the memory API. How often does that
happen, once per boot or more often? Are these registers accessed very
often by the guests?

>
>> >
>> > Andreas
>> >
>> > --
>> > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg