On 28 July 2015 at 11:27, Michael S. Tsirkin <mst at redhat.com> wrote:> On Tue, Jul 28, 2015 at 11:12:33AM +0100, Peter Maydell wrote: >> On 28 July 2015 at 11:08, Michael S. Tsirkin <mst at redhat.com> wrote: >> > On Tue, Jul 28, 2015 at 10:44:02AM +0100, Graeme Gregory wrote: >> >> Added the match table and pointers for ACPI probing to the driver. >> >> >> >> This uses the same identifier for virt devices as being used for qemu >> >> ARM64 ACPI support. >> >> >> >> http://git.linaro.org/people/shannon.zhao/qemu.git/commit/d0bf1955a3ecbab4b51d46f8c5dda02b7e14a17e >> >> >> >> Signed-off-by: Graeme Gregory <graeme.gregory at linaro.org> >> >> --- >> >> drivers/virtio/virtio_mmio.c | 10 ++++++++++ >> >> 1 file changed, 10 insertions(+) >> >> >> >> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c >> >> index 10189b5..f499d9d 100644 >> >> --- a/drivers/virtio/virtio_mmio.c >> >> +++ b/drivers/virtio/virtio_mmio.c >> >> @@ -58,6 +58,7 @@ >> >> >> >> #define pr_fmt(fmt) "virtio-mmio: " fmt >> >> >> >> +#include <linux/acpi.h> >> >> #include <linux/highmem.h> >> >> #include <linux/interrupt.h> >> >> #include <linux/io.h> >> >> @@ -732,12 +733,21 @@ static struct of_device_id virtio_mmio_match[] = { >> >> }; >> >> MODULE_DEVICE_TABLE(of, virtio_mmio_match); >> >> >> >> +#ifdef CONFIG_ACPI >> >> +static const struct acpi_device_id virtio_mmio_acpi_match[] = { >> >> + { "LNRO0005", }, >> >> + { } >> >> +}; >> > >> > Hmm - we have reserved QEMUXXXX in ASWG explicitly for this purpose. >> > >> > Pater - do you think it's a good idea to change this before QEMU 2.4 >> > is released? >> >> Shannon's call, I guess. I don't know enough about ACPI to say. >> I thought these ACPI IDs were already fixed because they were >> what the kernel was looking for... >> >> -- PMM > > Apparently not :) >We assigned LNRO in ASWG to avoid collisions with our prototypes/real platforms so it makes sense to me to switch to QEMUXXXX. I will submit a new patch if Shannon wants to switch to that form. Graeme
On 28 July 2015 at 11:33, G Gregory <graeme.gregory at linaro.org> wrote:> We assigned LNRO in ASWG to avoid collisions with our prototypes/real > platforms so it makes sense to me to switch to QEMUXXXX.So just to check, if we switch virtio-mmio from an LNRO0005 ID to a QEMUxxxx ID we aren't going to break any existing widely shipped or deployed code, right? If we can change the ID without breaking anything significant then I think the QEMU ID makes more sense; but it doesn't really gain us much beyond tidiness. PS: https://www.kernel.org/doc/Documentation/arm64/arm-acpi.txt uses virtio-mmio and LNRO0005 as its code example, so if we change this then it might be nice to update the docs as a followup. thanks -- PMM
On Wed, Jul 29, 2015 at 06:52:29PM +0100, Peter Maydell wrote:> On 28 July 2015 at 11:33, G Gregory <graeme.gregory at linaro.org> wrote: > > We assigned LNRO in ASWG to avoid collisions with our prototypes/real > > platforms so it makes sense to me to switch to QEMUXXXX. > > So just to check, if we switch virtio-mmio from an LNRO0005 ID > to a QEMUxxxx ID we aren't going to break any existing widely > shipped or deployed code, right? > > If we can change the ID without breaking anything significant > then I think the QEMU ID makes more sense; but it doesn't > really gain us much beyond tidiness. > > PS: https://www.kernel.org/doc/Documentation/arm64/arm-acpi.txt > uses virtio-mmio and LNRO0005 as its code example, so if > we change this then it might be nice to update the docs > as a followup. > > thanks > -- PMMSo this is the proposed patch. I agree it's merely about tidyness. Pls ack or nack - we need to decide before 2.4 is out. --> arm: change vendor ID for virtio-mmio Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index f365140..d10bd69 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -145,7 +145,7 @@ static void acpi_dsdt_add_virtio(Aml *scope, for (i = 0; i < num; i++) { Aml *dev = aml_device("VR%02u", i); - aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); + aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0005"))); aml_append(dev, aml_name_decl("_UID", aml_int(i))); Aml *crs = aml_resource_template();