This patch introduces a new PCI device which will act as the binding point
for Citrix branded PV drivers for Xen.
The intention is that Citrix Windows PV drivers will be available on Windows
Update and thus using the existing Xen platform PCI device as an anchor
point is not desirable as that device has been ubiquitous in HVM guests for
a long time and thus existing HVM guests running Windows would start
automatically downloading drivers from Windows Update when this may not be
desired by either the host or guest admin. This device therefore acts as
an opt-in for those wishing to deploy Citrix PV drivers.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
hw/i386/Makefile.objs | 1 +
hw/i386/citrix_pv_bus.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++
include/hw/pci/pci_ids.h | 3 ++
3 files changed, 126 insertions(+)
create mode 100644 hw/i386/citrix_pv_bus.c
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 205d22e..8e28831 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -4,3 +4,4 @@ obj-y += pc.o pc_piix.o pc_q35.o
obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
obj-y += kvmvapic.o
+obj-y += citrix_pv_bus.o
diff --git a/hw/i386/citrix_pv_bus.c b/hw/i386/citrix_pv_bus.c
new file mode 100644
index 0000000..e1e0508
--- /dev/null
+++ b/hw/i386/citrix_pv_bus.c
@@ -0,0 +1,122 @@
+/* Copyright (c) Citrix Systems Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms,
+ * with or without modification, are permitted provided
+ * that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer in the documentation and/or other
+ * materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "hw/hw.h"
+#include "hw/pci/pci.h"
+
+typedef struct _CitrixPVBusDevice {
+ PCIDevice dev;
+ uint8_t revision;
+ uint32_t pages;
+ MemoryRegion mmio;
+} CitrixPVBusDevice;
+
+static uint64_t citrix_pv_bus_mmio_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+ fprintf(stderr, "WARNING: read from address 0x" TARGET_FMT_plx
+ " in Citrix PV Bus MMIO BAR\n", addr);
+
+ return ~(uint64_t)0;
+}
+
+static void citrix_pv_bus_mmio_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+ fprintf(stderr, "WARNING: write to address 0x" TARGET_FMT_plx
+ " in Citrix PV Bus MMIO BAR\n", addr);
+}
+
+static const MemoryRegionOps citrix_pv_bus_mmio_ops = {
+ .read = &citrix_pv_bus_mmio_read,
+ .write = &citrix_pv_bus_mmio_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static int citrix_pv_bus_init(PCIDevice *pci_dev)
+{
+ CitrixPVBusDevice *d = DO_UPCAST(CitrixPVBusDevice, dev, pci_dev);
+ uint8_t *pci_conf;
+ uint64_t size;
+
+ pci_conf = pci_dev->config;
+
+ pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_MEMORY);
+ pci_set_byte(pci_conf + PCI_REVISION_ID, d->revision);
+
+ pci_config_set_prog_interface(pci_conf, 0);
+
+ pci_conf[PCI_INTERRUPT_PIN] = 1;
+
+ size = d->pages * TARGET_PAGE_SIZE;
+ memory_region_init_io(&d->mmio, &citrix_pv_bus_mmio_ops, d,
+ "citrix-bus-mmio", size);
+
+ pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_MEM_PREFETCH,
+ &d->mmio);
+
+ return 0;
+}
+
+static Property citrix_pv_bus_props[] = {
+ DEFINE_PROP_UINT8("revision", CitrixPVBusDevice, revision, 0x01),
+ DEFINE_PROP_UINT32("pages", CitrixPVBusDevice, pages, 128),
+ DEFINE_PROP_END_OF_LIST()
+};
+
+static void citrix_pv_bus_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+ k->init = citrix_pv_bus_init;
+ k->vendor_id = PCI_VENDOR_ID_CITRIX;
+ k->device_id = PCI_DEVICE_ID_CITRIX_PV_BUS;
+ k->class_id = PCI_CLASS_SYSTEM_OTHER;
+ k->subsystem_vendor_id = PCI_VENDOR_ID_CITRIX;
+ k->subsystem_id = PCI_DEVICE_ID_CITRIX_PV_BUS;
+ dc->desc = "Citrix PV Bus";
+ dc->props = citrix_pv_bus_props;
+}
+
+static const TypeInfo citrix_pv_bus_type_info = {
+ .name = "citrix-pv-bus",
+ .parent = TYPE_PCI_DEVICE,
+ .instance_size = sizeof(CitrixPVBusDevice),
+ .class_init = citrix_pv_bus_class_init,
+};
+
+static void citrix_pv_bus_register_types(void)
+{
+ type_register_static(&citrix_pv_bus_type_info);
+}
+
+type_init(citrix_pv_bus_register_types)
diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
index d8dc2f1..ed6a059 100644
--- a/include/hw/pci/pci_ids.h
+++ b/include/hw/pci/pci_ids.h
@@ -151,4 +151,7 @@
#define PCI_VENDOR_ID_TEWS 0x1498
#define PCI_DEVICE_ID_TEWS_TPCI200 0x30C8
+#define PCI_VENDOR_ID_CITRIX 0x5853
+#define PCI_DEVICE_ID_CITRIX_PV_BUS 0x0002
+
#endif
--
1.7.10.4
>>> On 02.07.13 at 10:39, Paul Durrant <paul.durrant@citrix.com> wrote: > --- a/include/hw/pci/pci_ids.h > +++ b/include/hw/pci/pci_ids.h > @@ -151,4 +151,7 @@ > #define PCI_VENDOR_ID_TEWS 0x1498 > #define PCI_DEVICE_ID_TEWS_TPCI200 0x30C8 > > +#define PCI_VENDOR_ID_CITRIX 0x5853Is that really the right way to do things, considering that the same value is elsewhere used for PCI_VENDOR_ID_XEN? Jan> +#define PCI_DEVICE_ID_CITRIX_PV_BUS 0x0002 > + > #endif
On Tue, 2013-07-02 at 09:39 +0100, Paul Durrant wrote:> This patch introduces a new PCI device which will act as the binding point > for Citrix branded PV drivers for Xen. > The intention is that Citrix Windows PV drivers will be available on Windows > Update and thus using the existing Xen platform PCI device as an anchor > point is not desirable as that device has been ubiquitous in HVM guests for > a long time and thus existing HVM guests running Windows would start > automatically downloading drivers from Windows Update when this may not be > desired by either the host or guest admin.What about <9AAE0902D5BC7E449B7C8E4E778ABCD00537EE@LONPEX01CL01.citrite.net> ?
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 02 July 2013 09:46 > To: Paul Durrant > Cc: xen-devel@lists.xen.org; qemu-devel@nongnu.org > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > >>> On 02.07.13 at 10:39, Paul Durrant <paul.durrant@citrix.com> wrote: > > --- a/include/hw/pci/pci_ids.h > > +++ b/include/hw/pci/pci_ids.h > > @@ -151,4 +151,7 @@ > > #define PCI_VENDOR_ID_TEWS 0x1498 > > #define PCI_DEVICE_ID_TEWS_TPCI200 0x30C8 > > > > +#define PCI_VENDOR_ID_CITRIX 0x5853 > > Is that really the right way to do things, considering that the same > value is elsewhere used for PCI_VENDOR_ID_XEN? > > Jan > > > +#define PCI_DEVICE_ID_CITRIX_PV_BUS 0x0002 > > + > > #endif >I opted to do this for clarity. The fact that the Xen platform device uses Citrix''s vendor ID is historical; I wanted to be clear that this device was distinct. Paul
On Tue, 2013-07-02 at 08:57 +0000, Paul Durrant wrote:> > -----Original Message----- > > From: Jan Beulich [mailto:JBeulich@suse.com] > > Sent: 02 July 2013 09:46 > > To: Paul Durrant > > Cc: xen-devel@lists.xen.org; qemu-devel@nongnu.org > > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > > > >>> On 02.07.13 at 10:39, Paul Durrant <paul.durrant@citrix.com> wrote: > > > --- a/include/hw/pci/pci_ids.h > > > +++ b/include/hw/pci/pci_ids.h > > > @@ -151,4 +151,7 @@ > > > #define PCI_VENDOR_ID_TEWS 0x1498 > > > #define PCI_DEVICE_ID_TEWS_TPCI200 0x30C8 > > > > > > +#define PCI_VENDOR_ID_CITRIX 0x5853 > > > > Is that really the right way to do things, considering that the same > > value is elsewhere used for PCI_VENDOR_ID_XEN? > > > > Jan > > > > > +#define PCI_DEVICE_ID_CITRIX_PV_BUS 0x0002 > > > + > > > #endif > > > > I opted to do this for clarity. The fact that the Xen platform device > uses Citrix''s vendor ID is historical;AFAIR this was XenSource''s vendor ID (it is "XS" in ASCII) which was donated to the Xen community. I''ll clarify this internally though.> I wanted to be clear that this device was distinct.I think giving two names to the same ID serves only to obfuscate. Ian.
> -----Original Message----- > From: Ian Campbell > Sent: 02 July 2013 10:02 > To: Paul Durrant > Cc: Jan Beulich; qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > On Tue, 2013-07-02 at 08:57 +0000, Paul Durrant wrote: > > > -----Original Message----- > > > From: Jan Beulich [mailto:JBeulich@suse.com] > > > Sent: 02 July 2013 09:46 > > > To: Paul Durrant > > > Cc: xen-devel@lists.xen.org; qemu-devel@nongnu.org > > > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > > > > > >>> On 02.07.13 at 10:39, Paul Durrant <paul.durrant@citrix.com> wrote: > > > > --- a/include/hw/pci/pci_ids.h > > > > +++ b/include/hw/pci/pci_ids.h > > > > @@ -151,4 +151,7 @@ > > > > #define PCI_VENDOR_ID_TEWS 0x1498 > > > > #define PCI_DEVICE_ID_TEWS_TPCI200 0x30C8 > > > > > > > > +#define PCI_VENDOR_ID_CITRIX 0x5853 > > > > > > Is that really the right way to do things, considering that the same > > > value is elsewhere used for PCI_VENDOR_ID_XEN? > > > > > > Jan > > > > > > > +#define PCI_DEVICE_ID_CITRIX_PV_BUS 0x0002 > > > > + > > > > #endif > > > > > > > I opted to do this for clarity. The fact that the Xen platform device > > uses Citrix''s vendor ID is historical; > > AFAIR this was XenSource''s vendor ID (it is "XS" in ASCII) which was > donated to the Xen community. I''ll clarify this internally though. >The PCI SIG has it registered to Citrix.> > I wanted to be clear that this device was distinct. > > I think giving two names to the same ID serves only to obfuscate. >Ok. I don''t mind dropping the definition if that''s generally preferred. Paul
> -----Original Message----- > From: Ian Campbell > Sent: 02 July 2013 09:57 > To: Paul Durrant > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > On Tue, 2013-07-02 at 09:39 +0100, Paul Durrant wrote: > > This patch introduces a new PCI device which will act as the binding point > > for Citrix branded PV drivers for Xen. > > The intention is that Citrix Windows PV drivers will be available on Windows > > Update and thus using the existing Xen platform PCI device as an anchor > > point is not desirable as that device has been ubiquitous in HVM guests for > > a long time and thus existing HVM guests running Windows would start > > automatically downloading drivers from Windows Update when this may > not be > > desired by either the host or guest admin. > > What about > <9AAE0902D5BC7E449B7C8E4E778ABCD00537EE@LONPEX01CL01.citrite.net> > ? >I had actually coded up a solution based on the existing Xen platform device, by having it synthesize a device ID based on the Xen version to which we could then host the xenbus driver, to allow us to deploy multiple versions of xenbus should compatibility (with things such as the shared info interface) become an issue. The co-installer for this driver could also spot existing PV driver installations and make sure they don''t get trashed. This idea was rejected by Citrix product teams though, because we would not be able to prevent any Windows guest without some known PV drivers from downloading our new driver from Windows Update and this was seen as undesirable. Paul
On Tue, 2013-07-02 at 10:14 +0100, Paul Durrant wrote:> > -----Original Message----- > > From: Ian Campbell > > Sent: 02 July 2013 09:57 > > To: Paul Durrant > > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org > > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > > > On Tue, 2013-07-02 at 09:39 +0100, Paul Durrant wrote: > > > This patch introduces a new PCI device which will act as the binding point > > > for Citrix branded PV drivers for Xen. > > > The intention is that Citrix Windows PV drivers will be available on Windows > > > Update and thus using the existing Xen platform PCI device as an anchor > > > point is not desirable as that device has been ubiquitous in HVM guests for > > > a long time and thus existing HVM guests running Windows would start > > > automatically downloading drivers from Windows Update when this may > > not be > > > desired by either the host or guest admin. > > > > What about > > <9AAE0902D5BC7E449B7C8E4E778ABCD00537EE@LONPEX01CL01.citrite.net> > > ? > > > > I had actually coded up a solution based on the existing Xen platform > device, by having it synthesize a device ID based on the Xen version > to which we could then host the xenbus driver, to allow us to deploy > multiple versions of xenbus should compatibility (with things such as > the shared info interface) become an issue. The co-installer for this > driver could also spot existing PV driver installations and make sure > they don''t get trashed.I think only this last bit of functionality is critical here, and it allows us to avoid having to carry multiple platform devices in upstream, doesn''t it?> This idea was rejected by Citrix product teams though, because we > would not be able to prevent any Windows guest without some known PV > drivers from downloading our new driver from Windows Update and this > was seen as undesirable.Well, if your product requirements are at odds with doing the right thing upstream then I think it would be best for you to just carry patches to make XS behave how you want. I hope we can find a suitable compromise though. Ian.
At 10:56 +0100 on 02 Jul (1372762607), Ian Campbell wrote:> On Tue, 2013-07-02 at 10:14 +0100, Paul Durrant wrote: > > I had actually coded up a solution based on the existing Xen platform > > device, by having it synthesize a device ID based on the Xen version > > to which we could then host the xenbus driver, to allow us to deploy > > multiple versions of xenbus should compatibility (with things such as > > the shared info interface) become an issue. The co-installer for this > > driver could also spot existing PV driver installations and make sure > > they don''t get trashed. > > I think only this last bit of functionality is critical here, and it > allows us to avoid having to carry multiple platform devices in > upstream, doesn''t it? > > > This idea was rejected by Citrix product teams though, because we > > would not be able to prevent any Windows guest without some known PV > > drivers from downloading our new driver from Windows Update and this > > was seen as undesirable. > > Well, if your product requirements are at odds with doing the right > thing upstream then I think it would be best for you to just carry > patches to make XS behave how you want.I think it''s a reasonable aim to have the WU drivers not spontaneously appear on VMs (on all Xen hosts, remember) where the admin has chosen not to install PV drivers. Generally, the more I think about this the more I''m convinced that _not_ installing the drivers on any existing systems without explicit permission is the most important thing.> I hope we can find a suitable compromise though.Well, the WU drivers could refuse to install except as upgrade to themselves (i.e. fail if there''s any unknown driver bound to the xen platform device, and also fail if there''s _no_ driver bound). Then the guest admin can choose to install the drivers by hand and get automatic updates after that. XS, XC and anyone else who chooses could carry a separate patch that changes the default to ''install if there are no drivers'', signalling over xenstore, or ACPI, or a Windows domain policy, or whatever. Tim.
On Tue, 2013-07-02 at 11:15 +0100, Tim Deegan wrote:> At 10:56 +0100 on 02 Jul (1372762607), Ian Campbell wrote: > > On Tue, 2013-07-02 at 10:14 +0100, Paul Durrant wrote: > > > I had actually coded up a solution based on the existing Xen platform > > > device, by having it synthesize a device ID based on the Xen version > > > to which we could then host the xenbus driver, to allow us to deploy > > > multiple versions of xenbus should compatibility (with things such as > > > the shared info interface) become an issue. The co-installer for this > > > driver could also spot existing PV driver installations and make sure > > > they don''t get trashed. > > > > I think only this last bit of functionality is critical here, and it > > allows us to avoid having to carry multiple platform devices in > > upstream, doesn''t it? > > > > > This idea was rejected by Citrix product teams though, because we > > > would not be able to prevent any Windows guest without some known PV > > > drivers from downloading our new driver from Windows Update and this > > > was seen as undesirable. > > > > Well, if your product requirements are at odds with doing the right > > thing upstream then I think it would be best for you to just carry > > patches to make XS behave how you want. > > I think it''s a reasonable aim to have the WU drivers not spontaneously > appear on VMs (on all Xen hosts, remember) where the admin has chosen > not to install PV drivers. > > Generally, the more I think about this the more I''m convinced that _not_ > installing the drivers on any existing systems without explicit > permission is the most important thing.Will WU install a completely fresh driver for a new (or indeed old) bit of hardware on native entirely without prompting? I''d have expected the old "Windows has found a driver for your device" dance.> > I hope we can find a suitable compromise though. > > Well, the WU drivers could refuse to install except as upgrade to > themselves (i.e. fail if there''s any unknown driver bound to the xen > platform device, and also fail if there''s _no_ driver bound). Then the > guest admin can choose to install the drivers by hand and get automatic > updates after that.That sounds reasonable. However I thought part of the point of getting things into WU was then that they could be "inbox" (either figuratively or literally) such that they would be installed by the Windows installer. Perhaps that''s a separate thing though.> XS, XC and anyone else who chooses could carry a separate patch that > changes the default to ''install if there are no drivers'', signalling > over xenstore, or ACPI, or a Windows domain policy, or whatever.Right.
> -----Original Message----- > From: Ian Campbell > Sent: 02 July 2013 11:24 > To: Tim (Xen.org) > Cc: Paul Durrant; qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > On Tue, 2013-07-02 at 11:15 +0100, Tim Deegan wrote: > > At 10:56 +0100 on 02 Jul (1372762607), Ian Campbell wrote: > > > On Tue, 2013-07-02 at 10:14 +0100, Paul Durrant wrote: > > > > I had actually coded up a solution based on the existing Xen platform > > > > device, by having it synthesize a device ID based on the Xen version > > > > to which we could then host the xenbus driver, to allow us to deploy > > > > multiple versions of xenbus should compatibility (with things such as > > > > the shared info interface) become an issue. The co-installer for this > > > > driver could also spot existing PV driver installations and make sure > > > > they don''t get trashed. > > > > > > I think only this last bit of functionality is critical here, and it > > > allows us to avoid having to carry multiple platform devices in > > > upstream, doesn''t it? > > > > > > > This idea was rejected by Citrix product teams though, because we > > > > would not be able to prevent any Windows guest without some known > PV > > > > drivers from downloading our new driver from Windows Update and > this > > > > was seen as undesirable. > > > > > > Well, if your product requirements are at odds with doing the right > > > thing upstream then I think it would be best for you to just carry > > > patches to make XS behave how you want. > > > > I think it''s a reasonable aim to have the WU drivers not spontaneously > > appear on VMs (on all Xen hosts, remember) where the admin has chosen > > not to install PV drivers. > > > > Generally, the more I think about this the more I''m convinced that _not_ > > installing the drivers on any existing systems without explicit > > permission is the most important thing. > > Will WU install a completely fresh driver for a new (or indeed old) bit > of hardware on native entirely without prompting? > > I''d have expected the old "Windows has found a driver for your device" > dance. > > > > I hope we can find a suitable compromise though. > > > > Well, the WU drivers could refuse to install except as upgrade to > > themselves (i.e. fail if there''s any unknown driver bound to the xen > > platform device, and also fail if there''s _no_ driver bound). Then the > > guest admin can choose to install the drivers by hand and get automatic > > updates after that. > > That sounds reasonable. However I thought part of the point of getting > things into WU was then that they could be "inbox" (either figuratively > or literally) such that they would be installed by the Windows > installer. Perhaps that''s a separate thing though. >No, that is the eventual aim so I don''t think the ''upgrade only'' options is really future-proof.> > XS, XC and anyone else who chooses could carry a separate patch that > > changes the default to ''install if there are no drivers'', signalling > > over xenstore, or ACPI, or a Windows domain policy, or whatever. > > Right. >Surely having a new device for the purposes of hosting Citrix PV drivers is a cleaner option for opting in? Note that I''m not proposing the new device displaces the existing platform device in any way. Paul
> -----Original Message----- > From: Ian Campbell > Sent: 02 July 2013 11:24 > To: Tim (Xen.org) > Cc: Paul Durrant; qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > On Tue, 2013-07-02 at 11:15 +0100, Tim Deegan wrote: > > At 10:56 +0100 on 02 Jul (1372762607), Ian Campbell wrote: > > > On Tue, 2013-07-02 at 10:14 +0100, Paul Durrant wrote: > > > > I had actually coded up a solution based on the existing Xen platform > > > > device, by having it synthesize a device ID based on the Xen version > > > > to which we could then host the xenbus driver, to allow us to deploy > > > > multiple versions of xenbus should compatibility (with things such as > > > > the shared info interface) become an issue. The co-installer for this > > > > driver could also spot existing PV driver installations and make sure > > > > they don''t get trashed. > > > > > > I think only this last bit of functionality is critical here, and it > > > allows us to avoid having to carry multiple platform devices in > > > upstream, doesn''t it? > > > > > > > This idea was rejected by Citrix product teams though, because we > > > > would not be able to prevent any Windows guest without some known > PV > > > > drivers from downloading our new driver from Windows Update and > this > > > > was seen as undesirable. > > > > > > Well, if your product requirements are at odds with doing the right > > > thing upstream then I think it would be best for you to just carry > > > patches to make XS behave how you want. > > > > I think it''s a reasonable aim to have the WU drivers not spontaneously > > appear on VMs (on all Xen hosts, remember) where the admin has chosen > > not to install PV drivers. > > > > Generally, the more I think about this the more I''m convinced that _not_ > > installing the drivers on any existing systems without explicit > > permission is the most important thing. > > Will WU install a completely fresh driver for a new (or indeed old) bit > of hardware on native entirely without prompting? >I believe it can be configured to do so.> I''d have expected the old "Windows has found a driver for your device" > dance. > > > > I hope we can find a suitable compromise though. > >I still think the extra device is the cleanest solution, and I have gone through all the alternatives I can think of. Paul
On Tue, 2013-07-02 at 11:31 +0100, Paul Durrant wrote:> > > XS, XC and anyone else who chooses could carry a separate patch that > > > changes the default to ''install if there are no drivers'', signalling > > > over xenstore, or ACPI, or a Windows domain policy, or whatever. > > > > Right. > > > > Surely having a new device for the purposes of hosting Citrix PV > drivers is a cleaner option for opting in? Note that I''m not proposing > the new device displaces the existing platform device in any way.A dislike for having to coordinate guest internal and guest external configuration changes in this way has been expressed by several people. However if you only intend to support your drivers on XenServer (and it is starting to seem like your constraints may end up forcing this option upon you) then you can equally well carry that new device in your patches too and manage it however your PMs require. Ian.
At 10:31 +0000 on 02 Jul (1372761105), Paul Durrant wrote:> > > Well, the WU drivers could refuse to install except as upgrade to > > > themselves (i.e. fail if there''s any unknown driver bound to the xen > > > platform device, and also fail if there''s _no_ driver bound). Then the > > > guest admin can choose to install the drivers by hand and get automatic > > > updates after that. > > > > That sounds reasonable. However I thought part of the point of getting > > things into WU was then that they could be "inbox" (either figuratively > > or literally) such that they would be installed by the Windows > > installer. Perhaps that''s a separate thing though. > > > > No, that is the eventual aim so I don''t think the ''upgrade only'' > options is really future-proof.Well, you can have them install by default on platforms that want it, or on new enough Xen versions. Or, even better, on new enough _windows_ versions.> > > XS, XC and anyone else who chooses could carry a separate patch that > > > changes the default to ''install if there are no drivers'', signalling > > > over xenstore, or ACPI, or a Windows domain policy, or whatever. > > > > Right. > > > > Surely having a new device for the purposes of hosting Citrix PV > drivers is a cleaner option for opting in?Only if it''s OK that the _host_ admin has to be involved (which was the original objection). Upgrade-only but hooked to the existing ID lets a guest admin install the drivers manually without plumbing it through $CLOUDPROVIDER''s toolstack, and without having it appear suddenly on existing VMs in the dead of night. If you could have it bind to either device then I guess a second PCI ID is another way of signalling that policy, but we should probably use one of the many other channels we have from the tools to the guest. Tim.
On Tue, Jul 02, 2013 at 11:48:46AM +0100, Ian Campbell wrote:> On Tue, 2013-07-02 at 11:31 +0100, Paul Durrant wrote: > > > > XS, XC and anyone else who chooses could carry a separate patch that > > > > changes the default to ''install if there are no drivers'', signalling > > > > over xenstore, or ACPI, or a Windows domain policy, or whatever. > > > > > > Right. > > > > > > > Surely having a new device for the purposes of hosting Citrix PV > > drivers is a cleaner option for opting in? Note that I''m not proposing > > the new device displaces the existing platform device in any way. > > A dislike for having to coordinate guest internal and guest external > configuration changes in this way has been expressed by several people. > > However if you only intend to support your drivers on XenServer (and it > is starting to seem like your constraints may end up forcing this option > upon you) then you can equally well carry that new device in your > patches too and manage it however your PMs require. >It''s also good to remember the Citrix XenServer Windows PV drivers are now opensource, and (more) easily usable on non-XenServer environments aswell. I hope they''d work on upstream Xen aswell. -- Pasi
On Tue, 2013-07-02 at 11:49 +0100, Tim Deegan wrote:> At 10:31 +0000 on 02 Jul (1372761105), Paul Durrant wrote: > > > > Well, the WU drivers could refuse to install except as upgrade to > > > > themselves (i.e. fail if there''s any unknown driver bound to the xen > > > > platform device, and also fail if there''s _no_ driver bound). Then the > > > > guest admin can choose to install the drivers by hand and get automatic > > > > updates after that. > > > > > > That sounds reasonable. However I thought part of the point of getting > > > things into WU was then that they could be "inbox" (either figuratively > > > or literally) such that they would be installed by the Windows > > > installer. Perhaps that''s a separate thing though. > > > > > > > No, that is the eventual aim so I don''t think the ''upgrade only'' > > options is really future-proof. > > Well, you can have them install by default on platforms that want it, or > on new enough Xen versions. Or, even better, on new enough _windows_ > versions. > > > > > XS, XC and anyone else who chooses could carry a separate patch that > > > > changes the default to ''install if there are no drivers'', signalling > > > > over xenstore, or ACPI, or a Windows domain policy, or whatever. > > > > > > Right. > > > > > > > Surely having a new device for the purposes of hosting Citrix PV > > drivers is a cleaner option for opting in? > > Only if it''s OK that the _host_ admin has to be involved (which was the > original objection). Upgrade-only but hooked to the existing ID lets a > guest admin install the drivers manually without plumbing it through > $CLOUDPROVIDER''s toolstack, and without having it appear suddenly on > existing VMs in the dead of night.I think part of the problem here is that it is unclear who the target audience for these drivers are. Paul, are you intending that these drivers be only for XenServer users or are you intending for them to be used by the broader community on a variety of different Xen platforms? Ian.
> -----Original Message----- > From: Ian Campbell > Sent: 02 July 2013 11:57 > To: Tim (Xen.org) > Cc: Paul Durrant; qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > On Tue, 2013-07-02 at 11:49 +0100, Tim Deegan wrote: > > At 10:31 +0000 on 02 Jul (1372761105), Paul Durrant wrote: > > > > > Well, the WU drivers could refuse to install except as upgrade to > > > > > themselves (i.e. fail if there''s any unknown driver bound to the xen > > > > > platform device, and also fail if there''s _no_ driver bound). Then the > > > > > guest admin can choose to install the drivers by hand and get > automatic > > > > > updates after that. > > > > > > > > That sounds reasonable. However I thought part of the point of getting > > > > things into WU was then that they could be "inbox" (either figuratively > > > > or literally) such that they would be installed by the Windows > > > > installer. Perhaps that''s a separate thing though. > > > > > > > > > > No, that is the eventual aim so I don''t think the ''upgrade only'' > > > options is really future-proof. > > > > Well, you can have them install by default on platforms that want it, or > > on new enough Xen versions. Or, even better, on new enough _windows_ > > versions. > > > > > > > XS, XC and anyone else who chooses could carry a separate patch that > > > > > changes the default to ''install if there are no drivers'', signalling > > > > > over xenstore, or ACPI, or a Windows domain policy, or whatever. > > > > > > > > Right. > > > > > > > > > > Surely having a new device for the purposes of hosting Citrix PV > > > drivers is a cleaner option for opting in? > > > > Only if it''s OK that the _host_ admin has to be involved (which was the > > original objection). Upgrade-only but hooked to the existing ID lets a > > guest admin install the drivers manually without plumbing it through > > $CLOUDPROVIDER''s toolstack, and without having it appear suddenly on > > existing VMs in the dead of night. > > I think part of the problem here is that it is unclear who the target > audience for these drivers are. > > Paul, are you intending that these drivers be only for XenServer users > or are you intending for them to be used by the broader community on a > variety of different Xen platforms? >My intention is that the drivers are widely available to all who want them, but the key word there is ''want''. No-one should get a surprise when we publish to Windows Update so having the drivers bind to a new device which can then be added to the VM config seems like the cleanest solution. I realise that this involves the VM provider having to do something to enable a VM to get drivers from Windows Update rather that the guest admin, but I think that''s actually the right way to do it. Adding this device into a VM is essentially enabling new functionality in the same way that adding a new network device or storage device would be. Paul
> -----Original Message----- > From: Tim Deegan [mailto:tim@xen.org] > Sent: 02 July 2013 11:50 > To: Paul Durrant > Cc: Ian Campbell; qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > At 10:31 +0000 on 02 Jul (1372761105), Paul Durrant wrote: > > > > Well, the WU drivers could refuse to install except as upgrade to > > > > themselves (i.e. fail if there''s any unknown driver bound to the xen > > > > platform device, and also fail if there''s _no_ driver bound). Then the > > > > guest admin can choose to install the drivers by hand and get automatic > > > > updates after that. > > > > > > That sounds reasonable. However I thought part of the point of getting > > > things into WU was then that they could be "inbox" (either figuratively > > > or literally) such that they would be installed by the Windows > > > installer. Perhaps that''s a separate thing though. > > > > > > > No, that is the eventual aim so I don''t think the ''upgrade only'' > > options is really future-proof. > > Well, you can have them install by default on platforms that want it, or > on new enough Xen versions. Or, even better, on new enough _windows_ > versions. > > > > > XS, XC and anyone else who chooses could carry a separate patch that > > > > changes the default to ''install if there are no drivers'', signalling > > > > over xenstore, or ACPI, or a Windows domain policy, or whatever. > > > > > > Right. > > > > > > > Surely having a new device for the purposes of hosting Citrix PV > > drivers is a cleaner option for opting in? > > Only if it''s OK that the _host_ admin has to be involved (which was the > original objection). Upgrade-only but hooked to the existing ID lets a > guest admin install the drivers manually without plumbing it through > $CLOUDPROVIDER''s toolstack, and without having it appear suddenly on > existing VMs in the dead of night. >But for new VMs we *want* the drivers to download and install automatically; we just don''t want them to do that for existing VMs. Paul
On Tue, 2013-07-02 at 13:35 +0100, Paul Durrant wrote:> > -----Original Message----- > > From: Ian Campbell > > Sent: 02 July 2013 11:57 > > To: Tim (Xen.org) > > Cc: Paul Durrant; qemu-devel@nongnu.org; xen-devel@lists.xen.org > > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > > > On Tue, 2013-07-02 at 11:49 +0100, Tim Deegan wrote: > > > At 10:31 +0000 on 02 Jul (1372761105), Paul Durrant wrote: > > > > > > Well, the WU drivers could refuse to install except as upgrade to > > > > > > themselves (i.e. fail if there''s any unknown driver bound to the xen > > > > > > platform device, and also fail if there''s _no_ driver bound). Then the > > > > > > guest admin can choose to install the drivers by hand and get > > automatic > > > > > > updates after that. > > > > > > > > > > That sounds reasonable. However I thought part of the point of getting > > > > > things into WU was then that they could be "inbox" (either figuratively > > > > > or literally) such that they would be installed by the Windows > > > > > installer. Perhaps that''s a separate thing though. > > > > > > > > > > > > > No, that is the eventual aim so I don''t think the ''upgrade only'' > > > > options is really future-proof. > > > > > > Well, you can have them install by default on platforms that want it, or > > > on new enough Xen versions. Or, even better, on new enough _windows_ > > > versions. > > > > > > > > > XS, XC and anyone else who chooses could carry a separate patch that > > > > > > changes the default to ''install if there are no drivers'', signalling > > > > > > over xenstore, or ACPI, or a Windows domain policy, or whatever. > > > > > > > > > > Right. > > > > > > > > > > > > > Surely having a new device for the purposes of hosting Citrix PV > > > > drivers is a cleaner option for opting in? > > > > > > Only if it''s OK that the _host_ admin has to be involved (which was the > > > original objection). Upgrade-only but hooked to the existing ID lets a > > > guest admin install the drivers manually without plumbing it through > > > $CLOUDPROVIDER''s toolstack, and without having it appear suddenly on > > > existing VMs in the dead of night. > > > > I think part of the problem here is that it is unclear who the target > > audience for these drivers are. > > > > Paul, are you intending that these drivers be only for XenServer users > > or are you intending for them to be used by the broader community on a > > variety of different Xen platforms? > > > > My intention is that the drivers are widely available to all who want > them, but the key word there is ''want''. No-one should get a surprise > when we publish to Windows Update so having the drivers bind to a new > device which can then be added to the VM config seems like the > cleanest solution.Actually, it occurs to me now (and I have a feeling Tim was trying to say this and I didn''t get it): It''s not really appropriate for any individual vendor to upload a driver to Windows Update which binds to the default platform device ID anyway. There are several other sets of Xen PV drivers out there (including the GPL PV drivers and various companies commercial/proprietary drivers) and having one particular set of drivers in WU puts all of them at a disadvantage. Before doing that we would need consensus behind the particular set of drivers, which I don''t think any of them currently have. All of which means that you do need your own device ID, for which you can upload support to WU. However I don''t think it therefore follows that you need that device ID to be supported by upstream. Does this work: We assign a device ID to your drivers (and we would do the same for any vendor). You can then upload drivers supporting that ID to WU and arrange within your product to create the appropriate device. At the same time you produce a setup.exe installer which will install those same drivers but also sets up (or includes) the binding to the existing device IDs. So anyone running on XenServer gets the driver direct from WU and you can define your own policies around what that means and what the upgrade path and ties to the platform mean etc. People who want to use your drivers on non-XenServer platforms can choose to do so by manually installing from within the guest, with no need to modify their guest configuration. Does that make sense? Ian.
> -----Original Message----- > From: Ian Campbell > Sent: 02 July 2013 13:44 > To: Paul Durrant > Cc: Tim (Xen.org); qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > On Tue, 2013-07-02 at 13:35 +0100, Paul Durrant wrote: > > > -----Original Message----- > > > From: Ian Campbell > > > Sent: 02 July 2013 11:57 > > > To: Tim (Xen.org) > > > Cc: Paul Durrant; qemu-devel@nongnu.org; xen-devel@lists.xen.org > > > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > > > > > On Tue, 2013-07-02 at 11:49 +0100, Tim Deegan wrote: > > > > At 10:31 +0000 on 02 Jul (1372761105), Paul Durrant wrote: > > > > > > > Well, the WU drivers could refuse to install except as upgrade to > > > > > > > themselves (i.e. fail if there''s any unknown driver bound to the > xen > > > > > > > platform device, and also fail if there''s _no_ driver bound). Then > the > > > > > > > guest admin can choose to install the drivers by hand and get > > > automatic > > > > > > > updates after that. > > > > > > > > > > > > That sounds reasonable. However I thought part of the point of > getting > > > > > > things into WU was then that they could be "inbox" (either > figuratively > > > > > > or literally) such that they would be installed by the Windows > > > > > > installer. Perhaps that''s a separate thing though. > > > > > > > > > > > > > > > > No, that is the eventual aim so I don''t think the ''upgrade only'' > > > > > options is really future-proof. > > > > > > > > Well, you can have them install by default on platforms that want it, or > > > > on new enough Xen versions. Or, even better, on new enough > _windows_ > > > > versions. > > > > > > > > > > > XS, XC and anyone else who chooses could carry a separate patch > that > > > > > > > changes the default to ''install if there are no drivers'', signalling > > > > > > > over xenstore, or ACPI, or a Windows domain policy, or whatever. > > > > > > > > > > > > Right. > > > > > > > > > > > > > > > > Surely having a new device for the purposes of hosting Citrix PV > > > > > drivers is a cleaner option for opting in? > > > > > > > > Only if it''s OK that the _host_ admin has to be involved (which was the > > > > original objection). Upgrade-only but hooked to the existing ID lets a > > > > guest admin install the drivers manually without plumbing it through > > > > $CLOUDPROVIDER''s toolstack, and without having it appear suddenly > on > > > > existing VMs in the dead of night. > > > > > > I think part of the problem here is that it is unclear who the target > > > audience for these drivers are. > > > > > > Paul, are you intending that these drivers be only for XenServer users > > > or are you intending for them to be used by the broader community on a > > > variety of different Xen platforms? > > > > > > > My intention is that the drivers are widely available to all who want > > them, but the key word there is ''want''. No-one should get a surprise > > when we publish to Windows Update so having the drivers bind to a new > > device which can then be added to the VM config seems like the > > cleanest solution. > > Actually, it occurs to me now (and I have a feeling Tim was trying to > say this and I didn''t get it): It''s not really appropriate for any > individual vendor to upload a driver to Windows Update which binds to > the default platform device ID anyway. > > There are several other sets of Xen PV drivers out there (including the > GPL PV drivers and various companies commercial/proprietary drivers) and > having one particular set of drivers in WU puts all of them at a > disadvantage. Before doing that we would need consensus behind the > particular set of drivers, which I don''t think any of them currently > have. > > All of which means that you do need your own device ID, for which you > can upload support to WU. However I don''t think it therefore follows > that you need that device ID to be supported by upstream. > > Does this work: > > We assign a device ID to your drivers (and we would do the same for any > vendor). You can then upload drivers supporting that ID to WU and > arrange within your product to create the appropriate device. > > At the same time you produce a setup.exe installer which will install > those same drivers but also sets up (or includes) the binding to the > existing device IDs. > > So anyone running on XenServer gets the driver direct from WU and you > can define your own policies around what that means and what the upgrade > path and ties to the platform mean etc. People who want to use your > drivers on non-XenServer platforms can choose to do so by manually > installing from within the guest, with no need to modify their guest > configuration. > > Does that make sense? >Yes, that makes sense *but* I would still like to avoid carrying a private patch to QEMU (and potentially have to keep rebasing it), hence my posting the patch the qemu-devel. Having the code in QEMU does no harm, clearly reserves the device id, and any VM provider (with a suitable version of QEMU on their host) can then enable the device should they wish. Am I missing something? Paul
--On 2 July 2013 13:43:38 +0100 Ian Campbell <Ian.Campbell@citrix.com> wrote:> We assign a device ID to your drivers (and we would do the same for any > vendor). You can then upload drivers supporting that ID to WU and > arrange within your product to create the appropriate device.Let''s say we do this, then Xirtic also produce PV drivers, and get assigned their own device ID. Will both of them be visible with a different device ID for the same device? Will windows cope with that? Or do we need to mediate which device IDs are exposed? -- Alex Bligh
On Tue, 2013-07-02 at 14:36 +0100, Alex Bligh wrote:> > --On 2 July 2013 13:43:38 +0100 Ian Campbell <Ian.Campbell@citrix.com> > wrote: > > > We assign a device ID to your drivers (and we would do the same for any > > vendor). You can then upload drivers supporting that ID to WU and > > arrange within your product to create the appropriate device. > > Let''s say we do this, then Xirtic also produce PV drivers, and get > assigned their own device ID. Will both of them be visible with > a different device ID for the same device?Different device ID for the same device isn''t possible, a device has exactly one device ID. I assume you mean two devices visible one with each ID?> Will windows cope with that? > Or do we need to mediate which device IDs are exposed?I''d expect that only Xirtic products would provide a device with that device ID. They may choose to make it mutually exclusive with the standard device, or they may choose to allow it to co-exist. In the latter case it is down to them what the behaviour of their drivers is. In any case this is all down to the people who make that product and their requirements etc. For people running Xirtic drivers on non-Xirtic products the rest of my mail applies -- IOW those users would use a setup.exe which binds those drivers to the normal device, there is no Xirtic device ID present anywhere in this scenario. Ian.
On Tue, 2013-07-02 at 13:51 +0100, Paul Durrant wrote:> Yes, that makes sense *but* I would still like to avoid carrying a > private patch to QEMU (and potentially have to keep rebasing it),It''s small and pretty self contained I think. Someone (Anthony?) recommended making it subclass the existing one, which ought to reduce the size of the patch to be maintained even further I think.> hence my posting the patch the qemu-devel. Having the code in QEMU > does no harm, clearly reserves the device id,FWIW I don''t think QEMU should be the registry of these device IDs. Can you create a file somewhere in the Xen source base to serve as the registry, probably somewhere under xen/include/public. We should probably have some sort of scheme. How about we declare the topmost available bit of the device id to be the "vendor specific" bit? We could split the dev id into N-bits of "vendor" and M-bits of device, or add a "locally administered" bit, but that might be overkill?> and any VM provider (with a suitable version of QEMU on their host) > can then enable the device should they wish.I imagine they would be worried about you pushing new drivers which depend on new XenServer features. So unless they also intend to implement (and track) your version compatibility xenstore keys (or whatever mechanism) I''m not sure why they would want to do such a thing. Or are you proposing to maintain forward and backward compatibility? i.e. based on feature flags and negotiation with backends rather than versioned platform devices or other XenServer specific mechanisms? In any case they could also apply the patch. If it turns out that lots of people are doing so then maybe that is the time to consider it for upstream. But in the meantime this avoids anyone other than XenServer having to think about policy and/or mechanism WRT multiple platform devices.> Am I missing something? > > Paul
> -----Original Message----- > From: Ian Campbell > Sent: 02 July 2013 14:43 > To: Paul Durrant > Cc: Tim (Xen.org); qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > On Tue, 2013-07-02 at 13:51 +0100, Paul Durrant wrote: > > Yes, that makes sense *but* I would still like to avoid carrying a > > private patch to QEMU (and potentially have to keep rebasing it), > > It''s small and pretty self contained I think. >It still has external interfaces, to tracing, QOM, etc that may change.> FWIW I don''t think QEMU should be the registry of these device IDs. Can > you create a file somewhere in the Xen source base to serve as the > registry, probably somewhere under xen/include/public.I''ll look at how best to do this.> > We should probably have some sort of scheme. How about we declare the > topmost available bit of the device id to be the "vendor specific" bit? > We could split the dev id into N-bits of "vendor" and M-bits of device, > or add a "locally administered" bit, but that might be overkill? > > > and any VM provider (with a suitable version of QEMU on their host) > > can then enable the device should they wish. > > I imagine they would be worried about you pushing new drivers which > depend on new XenServer features. So unless they also intend to > implement (and track) your version compatibility xenstore keys (or > whatever mechanism) I''m not sure why they would want to do such a thing.We already have people interested in doing this and we would look to QA in those non-XenServer environments, to ensure compatibility.> Or are you proposing to maintain forward and backward compatibility? > i.e. based on feature flags and negotiation with backends rather than > versioned platform devices or other XenServer specific mechanisms? >In general yes. Modifying the platform device revision is a mechanism of last resort (because of the inconvenience it causes) but one that we may still need at some point.> In any case they could also apply the patch. If it turns out that lots > of people are doing so then maybe that is the time to consider it for > upstream. >We already have interested parties and the device not being available upstream is an inconvenience. Paul
This patch introduces a new PCI device which will act as the binding point
for Citrix branded PV drivers for Xen.
The intention is that Citrix Windows PV drivers will be available on Windows
Update and thus using the existing Xen platform PCI device as an anchor
point is not desirable as that device has been ubiquitous in HVM guests for
a long time and thus existing HVM guests running Windows would start
automatically downloading drivers from Windows Update when this may not be
desired by either the host or guest admin. This device therefore acts as
an opt-in for those wishing to deploy Citrix PV drivers.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
hw/i386/Makefile.objs | 1 +
hw/i386/citrix_pv_bus.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++
include/hw/pci/pci_ids.h | 7 ++-
trace-events | 4 ++
4 files changed, 136 insertions(+), 2 deletions(-)
create mode 100644 hw/i386/citrix_pv_bus.c
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 205d22e..8e28831 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -4,3 +4,4 @@ obj-y += pc.o pc_piix.o pc_q35.o
obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
obj-y += kvmvapic.o
+obj-y += citrix_pv_bus.o
diff --git a/hw/i386/citrix_pv_bus.c b/hw/i386/citrix_pv_bus.c
new file mode 100644
index 0000000..c17cfa6
--- /dev/null
+++ b/hw/i386/citrix_pv_bus.c
@@ -0,0 +1,126 @@
+/* Copyright (c) Citrix Systems Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms,
+ * with or without modification, are permitted provided
+ * that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer in the documentation and/or other
+ * materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "hw/hw.h"
+#include "hw/pci/pci.h"
+#include "trace.h"
+
+#define TYPE_CITRIX_PV_BUS_DEVICE "citrix-pv"
+
+#define CITRIX_PV_BUS_DEVICE(obj) \
+ OBJECT_CHECK(CitrixPVBusDevice, (obj), TYPE_CITRIX_PV_BUS_DEVICE)
+
+typedef struct CitrixPVBusDevice {
+ /*< private >*/
+ PCIDevice parent_obj;
+ /*< public >*/
+ uint8_t revision;
+ uint32_t size;
+ MemoryRegion mmio;
+} CitrixPVBusDevice;
+
+static uint64_t citrix_pv_bus_mmio_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+ trace_citrix_pv_bus_mmio_read(addr);
+
+ return ~(uint64_t)0;
+}
+
+static void citrix_pv_bus_mmio_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+ trace_citrix_pv_bus_mmio_write(addr);
+}
+
+static const MemoryRegionOps citrix_pv_bus_mmio_ops = {
+ .read = &citrix_pv_bus_mmio_read,
+ .write = &citrix_pv_bus_mmio_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static int citrix_pv_bus_init(PCIDevice *pci_dev)
+{
+ CitrixPVBusDevice *d = CITRIX_PV_BUS_DEVICE(pci_dev);
+ uint8_t *pci_conf;
+
+ pci_conf = pci_dev->config;
+
+ pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_MEMORY);
+ pci_set_byte(pci_conf + PCI_REVISION_ID, d->revision);
+
+ pci_config_set_prog_interface(pci_conf, 0);
+
+ pci_conf[PCI_INTERRUPT_PIN] = 1;
+
+ memory_region_init_io(&d->mmio, &citrix_pv_bus_mmio_ops, d,
+ "citrix-bus-mmio", d->size);
+
+ pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_MEM_PREFETCH,
+ &d->mmio);
+
+ return 0;
+}
+
+static Property citrix_pv_bus_props[] = {
+ DEFINE_PROP_UINT8("revision", CitrixPVBusDevice, revision, 0x01),
+ DEFINE_PROP_UINT32("size", CitrixPVBusDevice, size, 0x400000),
+ DEFINE_PROP_END_OF_LIST()
+};
+
+static void citrix_pv_bus_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+ k->init = citrix_pv_bus_init;
+ k->vendor_id = PCI_VENDOR_ID_CITRIX;
+ k->device_id = PCI_DEVICE_ID_CITRIX_PV_BUS;
+ k->class_id = PCI_CLASS_SYSTEM_OTHER;
+ k->subsystem_vendor_id = PCI_VENDOR_ID_CITRIX;
+ k->subsystem_id = PCI_DEVICE_ID_CITRIX_PV_BUS;
+ dc->desc = "Citrix PV Bus";
+ dc->props = citrix_pv_bus_props;
+}
+
+static const TypeInfo citrix_pv_bus_type_info = {
+ .name = TYPE_CITRIX_PV_BUS_DEVICE,
+ .parent = TYPE_PCI_DEVICE,
+ .instance_size = sizeof(CitrixPVBusDevice),
+ .class_init = citrix_pv_bus_class_init,
+};
+
+static void citrix_pv_bus_register_types(void)
+{
+ type_register_static(&citrix_pv_bus_type_info);
+}
+
+type_init(citrix_pv_bus_register_types)
diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
index d8dc2f1..95b236f 100644
--- a/include/hw/pci/pci_ids.h
+++ b/include/hw/pci/pci_ids.h
@@ -142,8 +142,11 @@
#define PCI_DEVICE_ID_INTEL_Q35_MCH 0x29c0
-#define PCI_VENDOR_ID_XEN 0x5853
-#define PCI_DEVICE_ID_XEN_PLATFORM 0x0001
+#define PCI_VENDOR_ID_CITRIX 0x5853
+#define PCI_DEVICE_ID_CITRIX_PV_BUS 0x0002
+
+#define PCI_VENDOR_ID_XEN (PCI_VENDOR_ID_CITRIX)
+#define PCI_DEVICE_ID_XEN_PLATFORM 0x0001
#define PCI_VENDOR_ID_NEC 0x1033
#define PCI_DEVICE_ID_NEC_UPD720200 0x0194
diff --git a/trace-events b/trace-events
index c5f1ccb..63052b9 100644
--- a/trace-events
+++ b/trace-events
@@ -1161,3 +1161,7 @@ kvm_run_exit(int cpu_index, uint32_t reason)
"cpu_index %d, reason %d"
# qom/object.c
object_dynamic_cast_assert(const char *type, const char *target, const char
*file, int line, const char *func) "%s->%s (%s:%d:%s)"
object_class_dynamic_cast_assert(const char *type, const char *target, const
char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
+
+# hw/i386/citrix_pv_bus.c
+citrix_pv_bus_mmio_read(uint64_t addr) "WARNING: read from Citrix PV MMIO
space (address %"PRIx64")"
+citrix_pv_bus_mmio_write(uint64_t addr) "WARNING: write to Citrix PV MMIO
space (address %"PRIx64")"
--
1.7.10.4
> -----Original Message----- > > FWIW I don''t think QEMU should be the registry of these device IDs. Can > you create a file somewhere in the Xen source base to serve as the > registry, probably somewhere under xen/include/public. >Is the Xen source base actually the right place? The vendor ID belongs to Citrix. Does the PCI SIG hold a list of known device IDs for each vendor? If not then I guess we need some canonical place within Citrix to make sure device ids do not get re-used. Paul
On Tue, 2013-07-02 at 15:08 +0100, Paul Durrant wrote:> > -----Original Message----- > > > > FWIW I don''t think QEMU should be the registry of these device IDs. Can > > you create a file somewhere in the Xen source base to serve as the > > registry, probably somewhere under xen/include/public. > > > > Is the Xen source base actually the right place? The vendor ID belongs > to Citrix.So far as I know XenSource gave this ID over for use by the Xen project and nothing about the acquisition will have changed that. So although the ID may technically be owned by Citrix it is in effect managed by the Xen project and "belongs" to Xen. We need to clarify this internally.> Does the PCI SIG hold a list of known device IDs for each vendor? If > not then I guess we need some canonical place within Citrix to make > sure device ids do not get re-used. > > Paul
On 2 July 2013 15:03, Paul Durrant <paul.durrant@citrix.com> wrote:> This patch introduces a new PCI device which will act as the binding point > for Citrix branded PV drivers for Xen. > The intention is that Citrix Windows PV drivers will be available on Windows > Update and thus using the existing Xen platform PCI device as an anchor > point is not desirable as that device has been ubiquitous in HVM guests for > a long time and thus existing HVM guests running Windows would start > automatically downloading drivers from Windows Update when this may not be > desired by either the host or guest admin. This device therefore acts as > an opt-in for those wishing to deploy Citrix PV drivers.This commit message doesn''t really make the case for having this driver in upstream, IMHO. It sounds like it''s only of use for your branded product, which suggests that the best place for it is probably in your product, not upstream.> Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > --- > hw/i386/Makefile.objs | 1 + > hw/i386/citrix_pv_bus.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++hw/i386 is also not the right place for this. hw/misc/ would be better. thanks -- PMM
> -----Original Message----- > From: Peter Maydell [mailto:peter.maydell@linaro.org] > Sent: 02 July 2013 15:58 > To: Paul Durrant > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Qemu-devel] [PATCH] Citrix PV Bus device > > On 2 July 2013 15:03, Paul Durrant <paul.durrant@citrix.com> wrote: > > This patch introduces a new PCI device which will act as the binding point > > for Citrix branded PV drivers for Xen. > > The intention is that Citrix Windows PV drivers will be available on Windows > > Update and thus using the existing Xen platform PCI device as an anchor > > point is not desirable as that device has been ubiquitous in HVM guests for > > a long time and thus existing HVM guests running Windows would start > > automatically downloading drivers from Windows Update when this may > not be > > desired by either the host or guest admin. This device therefore acts as > > an opt-in for those wishing to deploy Citrix PV drivers. > > This commit message doesn''t really make the case for having this > driver in upstream, IMHO. It sounds like it''s only of use for your > branded product, which suggests that the best place for it is > probably in your product, not upstream. >No, that''s not true. The drivers themselves may be Citrix branded but they are not solely for use in Citrix branded VMs. Paul
Ian, --On 2 July 2013 14:42:41 +0100 Ian Campbell <Ian.Campbell@citrix.com> wrote:>> Let''s say we do this, then Xirtic also produce PV drivers, and get >> assigned their own device ID. Will both of them be visible with >> a different device ID for the same device? > > Different device ID for the same device isn''t possible, a device has > exactly one device ID. I assume you mean two devices visible one with > each ID?Yes. Two PCI devices talking to the same underlying thing.>> Will windows cope with that? >> Or do we need to mediate which device IDs are exposed? > > I''d expect that only Xirtic products would provide a device with that > device ID. They may choose to make it mutually exclusive with the > standard device, or they may choose to allow it to co-exist. In the > latter case it is down to them what the behaviour of their drivers is. > In any case this is all down to the people who make that product and > their requirements etc. > > For people running Xirtic drivers on non-Xirtic products the rest of my > mail applies -- IOW those users would use a setup.exe which binds those > drivers to the normal device, there is no Xirtic device ID present > anywhere in this scenario.OK. The thing I was suggesting we try to avoid was Citrix drivers binding to PCI-ID A and Xirtic drivers binding to PCI-ID B, and them both trying to work at once. Perhaps I should stop worrying about this as I''m finding it really hard to resist typing ''so don''t use Windows'' :-) -- Alex Bligh
On Tue, 2013-07-02 at 10:05, Paul Durrant wrote:> > -----Original Message----- > > From: Ian Campbell > > Sent: 02 July 2013 10:02 > > To: Paul Durrant > > Cc: Jan Beulich; qemu-devel@nongnu.org; xen-devel@lists.xen.org > > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > > > On Tue, 2013-07-02 at 08:57 +0000, Paul Durrant wrote: > > > > -----Original Message----- > > > > From: Jan Beulich [mailto:JBeulich@suse.com] > > > > Sent: 02 July 2013 09:46 > > > > To: Paul Durrant > > > > Cc: xen-devel@lists.xen.org; qemu-devel@nongnu.org > > > > Subject: Re: [Xen-devel] [PATCH] Citrix PV Bus device > > > > > > > > >>> On 02.07.13 at 10:39, Paul Durrant <paul.durrant@citrix.com> > wrote: > > > > > --- a/include/hw/pci/pci_ids.h > > > > > +++ b/include/hw/pci/pci_ids.h > > > > > @@ -151,4 +151,7 @@ > > > > > #define PCI_VENDOR_ID_TEWS 0x1498 > > > > > #define PCI_DEVICE_ID_TEWS_TPCI200 0x30C8 > > > > > > > > > > +#define PCI_VENDOR_ID_CITRIX 0x5853 > > > > > > > > Is that really the right way to do things, considering that the same > > > > value is elsewhere used for PCI_VENDOR_ID_XEN? > > > > > > > > Jan > > > > > > > > > +#define PCI_DEVICE_ID_CITRIX_PV_BUS 0x0002 > > > > > + > > > > > #endif > > > > > > > > > > I opted to do this for clarity. The fact that the Xen platform device > > > uses Citrix''s vendor ID is historical; > > > > AFAIR this was XenSource''s vendor ID (it is "XS" in ASCII) which was > > donated to the Xen community. I''ll clarify this internally though. > > > > The PCI SIG has it registered to Citrix.To be clear: the Xen PCI vendor ID (0x5853) is currently registered by Citrix on behalf of the Xen community. The ID is widely used in the community including within Linux and various vendor and community drivers for other OSs. Citrix has no intention to change this and will continue to fund this community resource through its PCI-SIG membership. Cheers, James (registered primary contact for Citrix''s PCI-SIG membership) -- James Bulpin Senior Director, Technology, XenServer Citrix
On 07/02/13 16:03, Paul Durrant wrote:> This patch introduces a new PCI device which will act as the binding point > for Citrix branded PV drivers for Xen. > The intention is that Citrix Windows PV drivers will be available on Windows > Update and thus using the existing Xen platform PCI device as an anchor > point is not desirable as that device has been ubiquitous in HVM guests for > a long time and thus existing HVM guests running Windows would start > automatically downloading drivers from Windows Update when this may not be > desired by either the host or guest admin. This device therefore acts as > an opt-in for those wishing to deploy Citrix PV drivers.How does this differ from the xen platform pci device, except for the fact that it has a different PCI ID? cheers, Gerd
> -----Original Message----- > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > Sent: 24 July 2013 11:20 > To: Paul Durrant > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org > Subject: Re: [Qemu-devel] [PATCH] Citrix PV Bus device > > On 07/02/13 16:03, Paul Durrant wrote: > > This patch introduces a new PCI device which will act as the binding point > > for Citrix branded PV drivers for Xen. > > The intention is that Citrix Windows PV drivers will be available on Windows > > Update and thus using the existing Xen platform PCI device as an anchor > > point is not desirable as that device has been ubiquitous in HVM guests for > > a long time and thus existing HVM guests running Windows would start > > automatically downloading drivers from Windows Update when this may > not be > > desired by either the host or guest admin. This device therefore acts as > > an opt-in for those wishing to deploy Citrix PV drivers. > > How does this differ from the xen platform pci device, except for the > fact that it has a different PCI ID? >This model doesn''t implement the fixed IO ports that the platform device does but he main point *is* the different PCI ID. This patch has been superseded by the Xen PV Device patch (see https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg00733.html) which generalizes the device to make the vendor ID, device ID and revision values configurable. Paul