From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH RFC] virtio-pci: new config layout: using memory BAR Date: Tue, 28 May 2013 13:53:35 -0500 Message-ID: <878v2zvsuo.fsf@codemonkey.ws> References: <20130528160342.GA29915@redhat.com> <87bo7vvxej.fsf@codemonkey.ws> <20130528173257.GC30296@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130528173257.GC30296@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: "Michael S. Tsirkin" Cc: Peter Maydell , kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, Stefan Hajnoczi , Paolo Bonzini , KONRAD Frederic List-Id: virtualization@lists.linuxfoundation.org "Michael S. Tsirkin" 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html