"Michael S. Tsirkin" <mst@xxxxxxxxxx> writes:
> On Tue, May 28, 2013 at 12:15:16PM -0500, Anthony Liguori wrote:
>> > @@ -455,6 +462,226 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
>> > }
>> > }
>> >
>> > +static uint64_t virtio_pci_config_common_read(void *opaque, hwaddr addr,
>> > + unsigned size)
>> > +{
>> > + VirtIOPCIProxy *proxy = opaque;
>> > + VirtIODevice *vdev = proxy->vdev;
>> > +
>> > + uint64_t low = 0xffffffffull;
>> > +
>> > + switch (addr) {
>> > + case offsetof(struct virtio_pci_common_cfg, device_feature_select):
>> > + return proxy->device_feature_select;
>>
>> Oh dear no... Please use defines like the rest of QEMU.
>
> Any good reason not to use offsetof?
> I see about 138 examples in qemu.
There are exactly zero:
$ find . -name "*.c" -exec grep -l "case offset" {} \;
$
> Anyway, that's the way Rusty wrote it in the kernel header -
> I started with defines.
> If you convince Rusty to switch I can switch too,
We have 300+ devices in QEMU that use #defines. We're not using this
kind of thing just because you want to copy code from the kernel.
>> https://github.com/aliguori/qemu/commit/587c35c1a3fe90f6af0f97927047ef4d3182a659
>>
>> And:
>>
>> https://github.com/aliguori/qemu/commit/01ba80a23cf2eb1e15056f82b44b94ec381565cb
>>
>> Which lets virtio-pci be subclassable and then remaps the config space to
>> BAR2.
>
>
> Interesting. Have the spec anywhere?
Not yet, but working on that.
> You are saying this is going to conflict because
> of BAR2 usage, yes?
No, this whole thing is flexible. I had to use BAR2 because BAR0 has to
be the vram mapping. It also had to be an MMIO bar.
The new layout might make it easier to implement a device like this. I
shared it mainly because I wanted to show the subclassing idea vs. just
tacking an option onto the existing virtio-pci code in QEMU.
Regards,
Anthony Liguori
> So let's only do this virtio-fb only for new layout, so we don't need
> to maintain compatibility. In particular, we are working
> on making memory BAR access fast for virtio devices
> in a generic way. At the moment they are measureably slower
> than PIO on x86.
>
>
>> Haven't looked at the proposed new ring layout yet.
>>
>> Regards,
>
> No new ring layout. It's new config layout.
>
>
> --
> 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
--
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
