From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rusty Russell Subject: Re: [PATCH RFC] virtio-pci: new config layout: using memory BAR Date: Wed, 29 May 2013 14:01:18 +0930 Message-ID: <87ppwammp5.fsf@rustcorp.com.au> References: <20130528160342.GA29915@redhat.com> <87bo7vvxej.fsf@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <87bo7vvxej.fsf@codemonkey.ws> 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: Anthony Liguori , virtualization@lists.linux-foundation.org, kvm@vger.kernel.org Cc: Peter Maydell , Paolo Bonzini , "Michael S. Tsirkin" , Stefan Hajnoczi , KONRAD Frederic List-Id: virtualization@lists.linuxfoundation.org Anthony Liguori writes: > "Michael S. Tsirkin" writes: >> + 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. It is pretty ugly. Yet the structure definitions are descriptive, capturing layout, size and endianness in natural a format readable by any C programmer. So AFAICT the question is, do we put the required #define VIRTIO_PCI_CFG_FEATURE_SEL \ (offsetof(struct virtio_pci_common_cfg, device_feature_select)) etc. in the kernel headers or qemu? > Haven't looked at the proposed new ring layout yet. No change, but there's an open question on whether we should nail it to little endian (or define the endian by the transport). Of course, I can't rule out that the 1.0 standard *may* decide to frob the ring layout somehow, but I'd think it would require a compelling reason. I suggest that's 2.0 material... Cheers, Rusty. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rusty Russell Subject: Re: [PATCH RFC] virtio-pci: new config layout: using memory BAR Date: Wed, 29 May 2013 14:01:18 +0930 Message-ID: <87ppwammp5.fsf@rustcorp.com.au> References: <20130528160342.GA29915@redhat.com> <87bo7vvxej.fsf@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Peter Maydell , Paolo Bonzini , "Michael S. Tsirkin" , Stefan Hajnoczi , KONRAD Frederic To: Anthony Liguori , "Michael S. Tsirkin" , virtualization@lists.linux-foundation.org, kvm@vger.kernel.org Return-path: In-Reply-To: <87bo7vvxej.fsf@codemonkey.ws> 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 List-Id: kvm.vger.kernel.org Anthony Liguori writes: > "Michael S. Tsirkin" writes: >> + 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. It is pretty ugly. Yet the structure definitions are descriptive, capturing layout, size and endianness in natural a format readable by any C programmer. So AFAICT the question is, do we put the required #define VIRTIO_PCI_CFG_FEATURE_SEL \ (offsetof(struct virtio_pci_common_cfg, device_feature_select)) etc. in the kernel headers or qemu? > Haven't looked at the proposed new ring layout yet. No change, but there's an open question on whether we should nail it to little endian (or define the endian by the transport). Of course, I can't rule out that the 1.0 standard *may* decide to frob the ring layout somehow, but I'd think it would require a compelling reason. I suggest that's 2.0 material... Cheers, Rusty.