Re: [PATCH v2 0/5] Add virtio-iommu driver

2018-06-28 Thread Peter Maydell 
On 27 June 2018 at 20:59, Michael S. Tsirkin  wrote:
> My point was virtio mmio isn't widely used outside of ARM.
> Rather than try to make everyone use it, IMHO it's better
> to start with PCI.

Yes. I would much rather we let virtio-mmio disappear into
history as a random bit of legacy stuff. One of the things
I didn't like about the virtio-iommu design was that it
resurrected virtio-mmio as something we need to actively care
about again, so if there is a path forward that will work with
pci virtio I would much prefer that.

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] KVM Forum 2016: Call For Participation

2016-04-26 Thread Peter Maydell 
On 10 March 2016 at 18:09, Paolo Bonzini  wrote:
> =
> KVM Forum 2016: Call For Participation
> August 24-26, 2016 - Westin Harbor Castle - Toronto, Canada
>
> (All submissions must be received before midnight May 1, 2016)
> =
>
> KVM Forum is an annual event that presents a rare opportunity
> for developers and users to meet, discuss the state of Linux
> virtualization technology, and plan for the challenges ahead.
> We invite you to lead part of the discussion by submitting a speaking
> proposal for KVM Forum 2016.

Hi; this is just a followup to remind people that the KVM Forum
CFP deadline of May 1st is rapidly approaching, so if you were
thinking of submitting a proposal now is the time.

All the CFP information is here:
http://events.linuxfoundation.org/events/kvm-forum/program/cfp

thanks
-- PMM (on behalf of the KVM Forum 2016 Program Committee)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH 3/7] linux-headers/kvm: add Hyper-V SynIC irq routing type and struct

2015-10-26 Thread Peter Maydell 
On 26 October 2015 at 09:50, Andrey Smetanin  wrote:
> Signed-off-by: Andrey Smetanin 
> Reviewed-by: Roman Kagan 
> Signed-off-by: Denis V. Lunev 
> CC: Vitaly Kuznetsov 
> CC: "K. Y. Srinivasan" 
> CC: Gleb Natapov 
> CC: Paolo Bonzini 
> CC: Roman Kagan 
> CC: Denis V. Lunev 
> CC: k...@vger.kernel.org
> CC: virtualization@lists.linux-foundation.org
>
> ---
>  linux-headers/linux/kvm.h | 8 
>  1 file changed, 8 insertions(+)

Hi. Changes to linux-headers/ should only be made as part of
an automated update from a mainline Linux kernel tree using
the scripts/update-linux-headers.sh script. This patch looks
like maybe it was a manual edit ?

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH 3/7] linux-headers/kvm: add Hyper-V SynIC irq routing type and struct

2015-10-26 Thread Peter Maydell 
On 26 October 2015 at 10:12, Denis V. Lunev <d...@openvz.org> wrote:
> On 10/26/2015 01:03 PM, Peter Maydell wrote:
>> Hi. Changes to linux-headers/ should only be made as part of
>> an automated update from a mainline Linux kernel tree using
>> the scripts/update-linux-headers.sh script. This patch looks
>> like maybe it was a manual edit ?

> yep. We know and have discussed this with Paolo already.
> Kernel stuff is in progress at the moment. The patch
> is presented to interested people to allow to compile and
> run.
>
> Actual merge will be performed with proper sync
> when kernel will be in rc3 or 4 stage and the patch will be
> dropped.

OK, cool. It's best to tag the series as 'RFC' rather than
'PATCH' in that case, as a reminder that it can't be applied
just yet.

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2] arm: change vendor ID for virtio-mmio

2015-07-31 Thread Peter Maydell 
On 29 July 2015 at 20:16, Michael S. Tsirkin m...@redhat.com wrote:
 ACPI spec 5.0 allows the use of PCI vendor IDs.

 Since we have one for virtio, it seems neater to use that
 rather than LNRO. For the device ID, use 103F which is a legacy ID that
 isn't used in virtio PCI spec - seems to make sense since virtio-mmio is
 a legacy device but we don't know the correct device type.

 Guests should probably match everything in the range 1000-103F
 (just like legacy pci drivers do) which will allow us to pass in the
 actual ID in the future if we want to.

 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/arm/virt-acpi-build.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
 index f365140..dea61ba 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(1AF4103F)));
  aml_append(dev, aml_name_decl(_UID, aml_int(i)));

So, I've just checked, and I believe that the kernel that RedHat
are shipping in their RHEL7 dev preview for AArch64 (and probably
thus also the Fedora/Centos one) includes a patch which adds
ACPI support to the virtio-mmio driver using the LNRO0005 ID string.

This to me suggests that we should just stick with that ID,
rather than changing to QEMU, the hex one based on the PCI
vendor ID, or anything else.

We're obviously under no obligation to make life easy for people
who ship kernels full of patches that haven't gone upstream yet,
but in this case there doesn't seem to me to be any benefit to
QEMU from picking an ID string that would break compatibility...

[The kernel I checked was the one in
https://git.centos.org/sources/kernel-aarch64/c7-aarch64/c589ab77889df6d93dbe817c373080631ab3275b
which despite the filename is actually an 80MB .tar.xz archive,
as pointed to by
https://git.centos.org/blob/rpms!kernel-aarch64/910dbce5f13419d68002f58e67ee6e762a93a425/.kernel-aarch64.metadata
]

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH v2] arm: change vendor ID for virtio-mmio

2015-07-30 Thread Peter Maydell 
On 30 July 2015 at 09:04, Michael S. Tsirkin m...@redhat.com wrote:
 On Thu, Jul 30, 2015 at 09:23:20AM +0800, Shannon Zhao wrote:

 Why do we drop the previous way using QEMU? Something I missed?

 So that guests that bind to this interface will work fine with non QEMU
 implementations of virtio-mmio.

I don't understand this sentence. If there are pre-existing
non-QEMU virtio-mmio implementations, then they're using
LNRO0005, and we should use it too. If there are going to
be implementations of virtio-mmio in future, then they will
use whatever identifier we pick here. Either way, we get
interoperability. I don't see any difference between our
saying the ID for virtio-mmio is QEMU0005 and saying
the ID for virtio-mmio is 1AF4103F.

(The latter seems unnecessarily opaque to me, to be honest.
At least an ID string QEMU gives you a clue where to
look for who owns the thing.)

Note also that strictly you don't mean non-QEMU implementations
of virtio-mmio, you mean non-QEMU implementations of the
ACPI tables. The hardware implementation of virtio-mmio
doesn't care at all about the ACPI ID. (In fact the most
plausible other-implementation would be UEFI using its
own (hard-coded) ACPI tables on top of a QEMU vexpress-a15
model or something similar.)

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_mmio: add ACPI probing

2015-07-29 Thread Peter Maydell 
On 28 July 2015 at 11:33, G Gregory graeme.greg...@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 QEMU.

So just to check, if we switch virtio-mmio from an LNRO0005 ID
to a QEMU 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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_mmio: add ACPI probing

2015-07-28 Thread Peter Maydell 
On 28 July 2015 at 11:08, Michael S. Tsirkin m...@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.greg...@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 QEMU 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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_mmio: add ACPI probing

2015-07-28 Thread Peter Maydell 
On 28 July 2015 at 11:27, Michael S. Tsirkin m...@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 m...@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.greg...@linaro.org

  +#ifdef CONFIG_ACPI
  +static const struct acpi_device_id virtio_mmio_acpi_match[] = {
  + { LNRO0005, },
  + { }
  +};
 
  Hmm - we have reserved QEMU 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...

 Apparently not :)

Mmm. I'm not terribly happy about stuff being in QEMU before the
ACPI spec for it has been finalised. We should not be picking
stuff randomly on the fly...

If we want to fix the ACPI IDs QEMU is using for 2.4 then we
really need to do that now (ie within the next day or two).

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_mmio: add ACPI probing

2015-07-28 Thread Peter Maydell 
On 28 July 2015 at 21:28, G Gregory graeme.greg...@linaro.org wrote:
 On 28 July 2015 at 21:12, Peter Maydell peter.mayd...@linaro.org wrote:
 Mmm. I'm not terribly happy about stuff being in QEMU before the
 ACPI spec for it has been finalised. We should not be picking
 stuff randomly on the fly...

 If we want to fix the ACPI IDs QEMU is using for 2.4 then we
 really need to do that now (ie within the next day or two).

 It is upto the owner of the QEMU prefix to allocate numbers. This is
 not an issue for ACPI spec at all.

I mean the specification for how this device should be advertised
in an ACPI table. I don't care whether that's an official ACPI
consortium thing or something less official. The table is
constructed by QEMU and read by the kernel (and possibly
also by UEFI?), so everybody needs to agree on what the
string is...

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH RFC 1/7] virtio: relax feature check

2015-05-12 Thread Peter Maydell 
On 12 May 2015 at 14:14, Cornelia Huck cornelia.h...@de.ibm.com wrote:
 On Wed, 06 May 2015 14:07:37 +0200
 Greg Kurz gk...@linux.vnet.ibm.com wrote:
 @@ -233,7 +233,6 @@ static inline void virtio_clear_feature(uint32_t 
 *features, unsigned int fbit)

  static inline bool __virtio_has_feature(uint32_t features, unsigned int 
 fbit)
  {
 -assert(fbit  32);
  return !!(features  (1  fbit));
  }




 I must say I'm not very comfortable with knowingly passing out-of-rage
 values to this function.

It would invoke C undefined behaviour, so clearly a bug if we did
pass an out-of-range value here. You'd need to at least do
if (fbit = 32) {
return false;
}
if you want to make it valid.

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: VirtIO-MMIO on MMU-less System

2014-10-28 Thread Peter Maydell 
On 28 October 2014 16:42, Christopher Covington c...@codeaurora.org wrote:
 Just out of curiosity, could VirtIO-MMIO peripherals work on an MMU-less
 system, such as a hypothetical M-flavor QEMU TCG virt machine?

No inherent reason, I think. If you care about M profile emulation
you should be aware that it's not currently very high quality,
though...

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [RFC PATCH 0/2] virtio-mmio: add irqfd support for vhost-net based on virtio-mmio

2014-10-27 Thread Peter Maydell 
On 25 October 2014 09:24, john.liuli john.li...@huawei.com wrote:
 To get the interrupt reason to support such VIRTIO_NET_F_STATUS
 features I add a new register offset VIRTIO_MMIO_ISRMEM which
 will help to establish a shared memory region between qemu and
 virtio-mmio device. Then the interrupt reason can be accessed by
 guest driver through this region. At the same time, the virtio-mmio
 dirver check this region to see irqfd is supported or not during
 the irq handler registration, and different handler will be assigned.

If you want to add a new register you should probably propose
an update to the virtio spec. However, it seems to me it would
be better to get generic PCI/PCIe working on the ARM virt
board instead; then we can let virtio-mmio quietly fade away.
This has been on the todo list for ages (and there have been
RFC patches posted for plain PCI), it's just nobody's had time
to work on it.

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH 1/2] virtio-gpu/2d: add hardware spec include file

2014-09-14 Thread Peter Maydell 
On 14 September 2014 06:46, Michael S. Tsirkin m...@redhat.com wrote:
 BUILD_BUG_ON in linux, QEMU_BUILD_BUG_ON in QEMU.
 You have to stick it in a C file though, so it
 won't be visible in this patch.

Why do you think that? We have several header files which
use QEMU_BUILD_BUG_ON and I don't see any reason why
it would have to be invoked from a .c file.

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH 1/2] virtio-gpu/2d: add hardware spec include file

2014-09-14 Thread Peter Maydell 
On 14 September 2014 07:11, Michael S. Tsirkin m...@redhat.com wrote:
 On Sun, Sep 14, 2014 at 07:04:11AM -0700, Peter Maydell wrote:
 On 14 September 2014 06:46, Michael S. Tsirkin m...@redhat.com wrote:
  BUILD_BUG_ON in linux, QEMU_BUILD_BUG_ON in QEMU.
  You have to stick it in a C file though, so it
  won't be visible in this patch.

 Why do you think that? We have several header files which
 use QEMU_BUILD_BUG_ON and I don't see any reason why
 it would have to be invoked from a .c file.

 Because Gerd wants to share this with linux uapi, and
 you can't use BUILD_BUG_ON in uapi headers on linux.

Who owns the master copy of the header and commits
to making sure it builds on other things than Linux+gcc
in that case?

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH 1/2] virtio-gpu/2d: add hardware spec include file

2014-09-14 Thread Peter Maydell 
On 14 September 2014 08:09, Michael S. Tsirkin m...@redhat.com wrote:
 On Sun, Sep 14, 2014 at 07:32:21AM -0700, Peter Maydell wrote:
 Who owns the master copy of the header and commits
 to making sure it builds on other things than Linux+gcc
 in that case?

 For most of virtio neither linux nor QEMU are the master.
 syncing them has been done manually in the past.
 As we are copying other headers from Linux anyway,
 I think it would be better for everyone if we make
 the Linux headers the master for QEMU going forward.

That's problematic because our copies of the linux headers
are only (and can only) be included in our include path
on Linux hosts. I had this problem with the PSCI headers
for ARM (I can't say I really like the solution I came up
with there, so if we can do better that would be cool).

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH 1/2] virtio-gpu/2d: add hardware spec include file

2014-09-12 Thread Peter Maydell 
On 12 September 2014 13:48, Eric Blake ebl...@redhat.com wrote:
 On 09/12/2014 04:44 AM, Gerd Hoffmann wrote:

 +enum virtgpu_ctrl_type {
 +VIRTGPU_UNDEFINED = 0,
 +
 +/* 2d commands */
 +VIRTGPU_CMD_GET_DISPLAY_INFO = 0x0100,

 Please consider also adding:

 #define VIRTGPU_CMD_GET_DISPLAY_INFO VIRTGPU_CMD_GET_DISPLAY_INFO

 and friends.  It makes it MUCH nicer for application software to probe
 for later extensions if every member of the enum is also associated with
 a preprocessor macro.

 I don't think this will ever be shipped as library header for external
 users ...

 Fair enough. I wasn't sure, so it didn't hurt to ask.


 +struct virtgpu_ctrl_hdr {
 +uint32_t type;
 +uint32_t flags;
 +uint64_t fence_id;
 +uint32_t ctx_id;
 +uint32_t padding;
 +};
 +

 Is the padding to ensure that this is aligned regardless of 32-bit or
 64-bit hosts?

 Yes.

 Is it worth adding a compile-time assertion about the
 size of the struct to ensure the compiler doesn't add any additional
 padding?

 Makes sense.  What is the usual trick to do that?

 Among others,

 struct dummy {
   int size_check : (sizeof(virtgpu_ctrl_hdr) == 24 ? 1 : -1);
 };

QEMU already has a BUILD_BUG_ON() macro, so you can just say

QEMU_BUILD_BUG_ON(sizeof(virtgpu_ctrl_hdr) != 24);

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [Qemu-devel] [PATCH 1/2] virtio-gpu/2d: add hardware spec include file

2014-09-11 Thread Peter Maydell 
On 11 September 2014 16:09, Gerd Hoffmann kra...@redhat.com wrote:
 This patch adds the header file with structs and defines for
 the virtio based gpu device.  Covers 2d operations only.

 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---
  include/hw/virtio/virtgpu_hw.h | 158 
 +
  1 file changed, 158 insertions(+)
  create mode 100644 include/hw/virtio/virtgpu_hw.h

 diff --git a/include/hw/virtio/virtgpu_hw.h b/include/hw/virtio/virtgpu_hw.h
 new file mode 100644
 index 000..461f452
 --- /dev/null
 +++ b/include/hw/virtio/virtgpu_hw.h
 @@ -0,0 +1,158 @@
 +#ifndef VIRTGPU_HW_H
 +#define VIRTGPU_HW_H
 +
 +enum virtgpu_ctrl_type {
 +VIRTGPU_UNDEFINED = 0,


This is clearly all well out of line with our
coding style guide, which isn't a terribly good start...

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: what should a virtio-mmio transport without a backend look like?

2013-06-24 Thread Peter Maydell 
On 24 June 2013 13:26, Christopher Covington c...@codeaurora.org wrote:
 On 06/22/2013 06:51 AM, Peter Maydell wrote:
 On 21 June 2013 19:45, Christopher Covington c...@codeaurora.org wrote:
 You were proposing to use a valid/existing MagicValue/Version/VendorID with 
 a
 special DeviceID that does nothing. I'm saying why not use a valid/existing
 MagicValue/Version/VendorID/DeviceID with a special parameter setting, 
 size=0,
 that does nothing?

 [...]

 Also, it's mixing a detail of the backend layer (what
 a zero-sized disk happens to look like) with the transport layer,
 which seems a bit ugly spec-wise.

 I don't think that has to be the case. From what I understand of your
 architecture, the device layer is completely opaque to the transport layer,
 and the transport layer is immutable, but surely the device layer will at
 least know how many transports are available? As long as that's true, can't
 the device layer just create real devices and hook them up to transports, and
 then create no-op devices and hook them up to any remaining transports?

No, the device creation code just creates all the devices the user
asks for. At the point where it can't find a transport to
plug one into, the device-creation fails with an error message.
It doesn't attempt to find buses with nothing plugged in (since
for instance it's entirely reasonable to have a PCI slot with
no card in it).

 (Implementation wise I'm not crazy about it either since it would
 be way more complicated than saying no backend? OK, RAZ/WI.)

 (I thought virtio block devices were already implemented.)

Yes, but what does a transport do if there's no backend is
transport code, whereas the block device is backend code.
I actually think the creation of a spurious /dev/vda makes
'zero-sized block backend' a non-starter anyway, though.
It's also neither conceptually pure nor pragmatically easy,
so it falls between two stools.

I could see the rationale for a 'cleanliness of spec' position
that we should handle this with a 'null' backend with a new
DeviceID which is completely functional (including all the host
feature bitmaps, setting up queues, interrupts, etc) but just
doesn't do anything. That would be more complicated to implement
than RAZ/WI, but not impossible.

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: what should a virtio-mmio transport without a backend look like?

2013-06-22 Thread Peter Maydell 
On 21 June 2013 19:45, Christopher Covington c...@codeaurora.org wrote:
 You were proposing to use a valid/existing MagicValue/Version/VendorID with a
 special DeviceID that does nothing. I'm saying why not use a valid/existing
 MagicValue/Version/VendorID/DeviceID with a special parameter setting, size=0,
 that does nothing?

Ah, I see. Well, it sounds from your quoted mount message as if they
do actually do something (ie the kernel finds a block device and
creates a /dev/vda for it), rather than nothing, which seems like
a bad thing. Also, it's mixing a detail of the backend layer (what
a zero-sized disk happens to look like) with the transport layer,
which seems a bit ugly spec-wise.

(Implementation wise I'm not crazy about it either since it would
be way more complicated than saying no backend? OK, RAZ/WI.)

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: what should a virtio-mmio transport without a backend look like?

2013-06-21 Thread Peter Maydell 
On 20 June 2013 13:58, Christopher Covington c...@codeaurora.org wrote:
 On Thu, 2013-06-20 at 11:29 +0100, Peter Maydell wrote:
 1. One question I've run into is: what should a virtio-mmio transport
 with no backend look like to the guest OS? The spec as written
 seems to assume that there's always some backend present.
 (The idea is that QEMU might just always instantiate say 8
 mmio transports, and then whether they actually have a
 blk/net/whatever backend depends on user options).

 Might it be reasonably easy to just not enumerate unused transports
 in the device tree or kernel parameters?

At least for QEMU, the backend that plugs into the transport
isn't created until after the machine model has created
transports and put together the device tree blob. So we don't
really have the information about what devices are going to
appear at the point we're doing this.

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: what should a virtio-mmio transport without a backend look like?

2013-06-21 Thread Peter Maydell 
On 21 June 2013 17:47, Pawel Moll pawel.m...@arm.com wrote:
 On Fri, 2013-06-21 at 17:41 +0100, Peter Maydell wrote:
 As it happens, if you use the command line to specify
 a virtio device it doesn't make the same complaint about
 bad magic number as if you specify it via dtb, but that
 should probably be fixed in the kernel :-)

 I don't really see how this would be possible - the complaining code
 is just a normal platform device probe function.

Sorry, you're correct and I misremembered. I just retested
with specifying via command line and it behaves the same
way as via dtb.

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: what should a virtio-mmio transport without a backend look like?

2013-06-21 Thread Peter Maydell 
On 21 June 2013 19:01, Christopher Covington c...@codeaurora.org wrote:
 Anyhow, I just did a quick experiment with 0-size block devices, and they seem
 to work for me, although trying to mount the device yields the confusing
 message, mount: mounting /dev/vda on mount failed: Invalid argument.

I'm confused -- what's the significance of zero size
block devices?

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC] virtio-pci: new config layout: using memory BAR

2013-05-29 Thread Peter Maydell 
On 29 May 2013 09:24, Michael S. Tsirkin m...@redhat.com wrote:
 diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
 index f4db224..fd09ea7 100644
 --- a/hw/virtio/virtio-pci.c
 +++ b/hw/virtio/virtio-pci.c
 @@ -467,51 +467,70 @@ static uint64_t virtio_pci_config_common_read(void 
 *opaque, hwaddr addr,
  {
  VirtIOPCIProxy *proxy = opaque;
  VirtIODevice *vdev = proxy-vdev;
 +struct virtio_pci_common_cfg cfg;

  uint64_t low = 0xull;

  switch (addr) {
  case offsetof(struct virtio_pci_common_cfg, device_feature_select):
 +assert(size == sizeof cfg.device_feature_select);
  return proxy-device_feature_select;

Asserting is definitely the wrong thing here, since the
guest can trigger it.

If you really want to use offsetof like this you're
going to need to decorate the structs with QEMU_PACKED.

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC] virtio-pci: new config layout: using memory BAR

2013-05-29 Thread Peter Maydell 
On 29 May 2013 11:08, Michael S. Tsirkin m...@redhat.com wrote:
 On Wed, May 29, 2013 at 10:00:33AM +0100, Peter Maydell wrote:
 Asserting is definitely the wrong thing here, since the
 guest can trigger it.

 So?

So guest should not be able to crash QEMU is a standard
rule: assert() is for QEMU bugs, not guest bugs.
Virtio isn't any different to any other device model.

 It's a driver bug. It can reset or crash guest with the same effect,
 and it likely will if we let it continue.

Letting a misbehaving guest crash itself is fine. Killing
QEMU isn't.

 assert makes sure we don't let it escalate into some
 hard to debug security problem.

If you want to make guest bugs easier to spot and debug
this is what qemu_log_mask(LOG_GUEST_ERROR,...) is for.

 If you really want to use offsetof like this you're
 going to need to decorate the structs with QEMU_PACKED.

 Nope.
 These structs are carefully designed not to have any padding.

...on every compiler and OS combination that QEMU builds for?

 And if there was a bug and there was some padding, we still
 can't fix it with PACKED because this structure
 is used to interact with the guest code which does not
 have the packed attribute.

The guest code has to use a set of structure offsets and
sizes which is fixed by the virtio ABI -- how it implements
this is up to the guest (and if it misimplements it that is
a guest bug and not our problem). Note also that the ABI
is not defined by a set of C source struct definitions
(except in as far as they are accompanied by a set of
restrictions on alignment, padding, etc that completely
determine the numerical alignments and offsets).
How QEMU implements the set of offsets and sizes specified
by the ABI is definitely our problem, and is exactly what
this discussion is about. The simplest and safest way to
get the offsets right on all platforms is just to use a set
of #defines, which is why that's what almost all of our
device models do. Where we do define a struct matching
guest memory layout, we tag it with QEMU_PACKED as our
best means of ensuring consistency on all hosts.

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC] virtio-pci: new config layout: using memory BAR

2013-05-29 Thread Peter Maydell 
On 29 May 2013 14:24, Michael S. Tsirkin m...@redhat.com wrote:
 On Wed, May 29, 2013 at 07:52:37AM -0500, Anthony Liguori wrote:
 1) C makes no guarantees about structure layout beyond the first
member.  Yes, if it's naturally aligned or has a packed attribute,
GCC does the right thing.  But this isn't kernel land anymore,
portability matters and there are more compilers than GCC.

 You expect a compiler to pad this structure:

 struct foo {
 uint8_t a;
 uint8_t b;
 uint16_t c;
 uint32_t d;
 };

 I'm guessing any compiler that decides to waste memory in this way
 will quickly get dropped by users and then we won't worry
 about building QEMU with it.

Structure alignment is a platform ABI choice. Oddly enough people
choose operating systems on more criteria than the amount of
padding their ABI mandates in structures.

In any case it's really tedious to read a structure and wade
through working out whether it's going to have all its members
naturally aligned, especially once it's more than a handful
of members in size. And getting it wrong is silent portability
problem.

thanks
-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: Additional virtio-mmio spec changes

2011-11-14 Thread Peter Maydell 
Rusty wrote:
Pawel Moll pawel.m...@arm.com wrote:
 Peter also mentioned that he didn't like the Num in
 QueueNum register name and would rather see it called QueueSize. I
 have no strong opinions either way and can provide patches to both the
 spec and the driver (and header) to change it. Any thoughts?

 Num isn't great, but Size might be misleading.  I'd leave it...

I'd note that calling it QueueNum  is inconsistent with the main
virtio spec, which calls the equivalent virtio header field
Queue Size. Also if the docs say

QueueNum
 Virtual queue size.
 Queue size is a number of elements in the queue [...]

this rather suggests to me that we've picked the wrong name for
the register...

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC] virtio-spec: flexible configuration layout

2011-11-14 Thread Peter Maydell 
On 9 November 2011 11:06, Pawel Moll pawel.m...@arm.com wrote:
 On Wed, 2011-11-09 at 10:55 +, Sasha Levin wrote:
 On Wed, 2011-11-09 at 10:47 +, Pawel Moll wrote:
  Yep, it's actually already in 3.2-rc1 (drivers/virtio/virtio_mmio.c) and
  in the spec (see Appendix X). And actually the control registers layout
  I used was originally based on the PCI legacy headers (slightly
  simplified), but evolved a bit since. My understanding is that the
  changes Michael is proposing affect the PCI device interface only so
  they shouldn't affect my interface.

 I didn't know it's in already, might be interesting adding support to it
 to x86 userspace tools.

 Do you mean the qemu or the (non-qemu) KVM tools I know nothing
 about? ;-)

 If qemu, Peter got it already running with virtio_mmio on ARM models
 (not upstream yet as far as I know), but I presume the code would we
 mostly non-ARM specific.

The QEMU code is completely non-ARM-specific (apart from the line
in the relevant ARM board models where we instantiate the devices).
I haven't submitted it upstream yet though because really the
virtio transport layer in QEMU needs to be refactored as a qdev
bus first.

-- PMM
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization