* [PATCH 2/2] kvm: qemu: check device assignment command
@ 2009-03-26 9:02 Han, Weidong
2009-03-26 12:04 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Han, Weidong @ 2009-03-26 9:02 UTC (permalink / raw)
To: 'Avi Kivity'; +Cc: 'kvm@vger.kernel.org'
[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]
Device assignment command is like "-pcidevice host=xx:yy.z".
Check bus:dev.func length to make sure its format is xx:yy.z.
Signed-off-by: Weidong Han <weidong.han@intel.com>
---
qemu/hw/device-assignment.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index cef7c8a..50a0d5c 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -1196,7 +1196,7 @@ out:
AssignedDevInfo *add_assigned_device(const char *arg)
{
char *cp, *cp1;
- char device[8];
+ char device[9];
char dma[6];
int r;
AssignedDevInfo *adev;
@@ -1207,6 +1207,9 @@ AssignedDevInfo *add_assigned_device(const char *arg)
return NULL;
}
r = get_param_value(device, sizeof(device), "host", arg);
+ /* b:d.f format: xx:yy.z */
+ if (r != 7)
+ goto bad;
r = get_param_value(adev->name, sizeof(adev->name), "name", arg);
if (!r)
snprintf(adev->name, sizeof(adev->name), "%s", device);
--
1.6.0.4
[-- Attachment #2: 0002-kvm-qemu-check-device-assignment-command.patch --]
[-- Type: application/octet-stream, Size: 1251 bytes --]
From 7ce0e9784ec2d9743fce28537c3369d924b33a90 Mon Sep 17 00:00:00 2001
From: Weidong Han <weidong.han@intel.com>
Date: Fri, 27 Mar 2009 00:53:06 +0800
Subject: [PATCH] kvm: qemu: check device assignment command
Device assignment command is like "-pcidevice host=xx:yy.z".
Check bus:dev.func length to make sure its format is xx:yy.z.
Signed-off-by: Weidong Han <weidong.han@intel.com>
---
qemu/hw/device-assignment.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index cef7c8a..50a0d5c 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -1196,7 +1196,7 @@ out:
AssignedDevInfo *add_assigned_device(const char *arg)
{
char *cp, *cp1;
- char device[8];
+ char device[9];
char dma[6];
int r;
AssignedDevInfo *adev;
@@ -1207,6 +1207,9 @@ AssignedDevInfo *add_assigned_device(const char *arg)
return NULL;
}
r = get_param_value(device, sizeof(device), "host", arg);
+ /* b:d.f format: xx:yy.z */
+ if (r != 7)
+ goto bad;
r = get_param_value(adev->name, sizeof(adev->name), "name", arg);
if (!r)
snprintf(adev->name, sizeof(adev->name), "%s", device);
--
1.6.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] kvm: qemu: check device assignment command
2009-03-26 9:02 [PATCH 2/2] kvm: qemu: check device assignment command Han, Weidong
@ 2009-03-26 12:04 ` Avi Kivity
2009-03-27 9:31 ` Han, Weidong
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2009-03-26 12:04 UTC (permalink / raw)
To: Han, Weidong; +Cc: 'kvm@vger.kernel.org'
Han, Weidong wrote:
> Device assignment command is like "-pcidevice host=xx:yy.z".
> Check bus:dev.func length to make sure its format is xx:yy.z.
>
> Signed-off-by: Weidong Han <weidong.han@intel.com>
> ---
> qemu/hw/device-assignment.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
> index cef7c8a..50a0d5c 100644
> --- a/qemu/hw/device-assignment.c
> +++ b/qemu/hw/device-assignment.c
> @@ -1196,7 +1196,7 @@ out:
> AssignedDevInfo *add_assigned_device(const char *arg)
> {
> char *cp, *cp1;
> - char device[8];
> + char device[9];
> char dma[6];
> int r;
> AssignedDevInfo *adev;
> @@ -1207,6 +1207,9 @@ AssignedDevInfo *add_assigned_device(const char *arg)
> return NULL;
> }
> r = get_param_value(device, sizeof(device), "host", arg);
> + /* b:d.f format: xx:yy.z */
> + if (r != 7)
> + goto bad;
> r = get_param_value(adev->name, sizeof(adev->name), "name", arg);
> if (!r)
> snprintf(adev->name, sizeof(adev->name), "%s", device);
>
I suggest replacing the parsing code with pci_parse_devaddr() (needs to
be extended to support functions) so that all the checking and parsing
is done in one place.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 2/2] kvm: qemu: check device assignment command
2009-03-26 12:04 ` Avi Kivity
@ 2009-03-27 9:31 ` Han, Weidong
2009-03-29 14:35 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Han, Weidong @ 2009-03-27 9:31 UTC (permalink / raw)
To: 'Avi Kivity'; +Cc: 'kvm@vger.kernel.org'
Avi Kivity wrote:
> Han, Weidong wrote:
>> Device assignment command is like "-pcidevice host=xx:yy.z".
>> Check bus:dev.func length to make sure its format is xx:yy.z.
>>
>> Signed-off-by: Weidong Han <weidong.han@intel.com>
>> ---
>> qemu/hw/device-assignment.c | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu/hw/device-assignment.c
>> b/qemu/hw/device-assignment.c index cef7c8a..50a0d5c 100644 ---
>> a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c
>> @@ -1196,7 +1196,7 @@ out:
>> AssignedDevInfo *add_assigned_device(const char *arg) {
>> char *cp, *cp1;
>> - char device[8];
>> + char device[9];
>> char dma[6];
>> int r;
>> AssignedDevInfo *adev;
>> @@ -1207,6 +1207,9 @@ AssignedDevInfo *add_assigned_device(const
>> char *arg) return NULL; }
>> r = get_param_value(device, sizeof(device), "host", arg);
>> + /* b:d.f format: xx:yy.z */
>> + if (r != 7)
>> + goto bad;
>> r = get_param_value(adev->name, sizeof(adev->name), "name",
>> arg); if (!r) snprintf(adev->name, sizeof(adev->name), "%s",
>> device);
>>
>
> I suggest replacing the parsing code with pci_parse_devaddr() (needs
> to be extended to support functions) so that all the checking and
> parsing is done in one place.
If use pci_parse_devaddr(), it needs to add domain section to assigning command, and add function section to pci_add/pci_del commands. What's more, pci_parse_devaddr() parses guest device bdf, there are some assumption, such as function is 0. But here parse host bdf. It's a little complex to combine them together.
Regards,
Weidong
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] kvm: qemu: check device assignment command
2009-03-27 9:31 ` Han, Weidong
@ 2009-03-29 14:35 ` Avi Kivity
2009-03-30 12:41 ` Han, Weidong
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2009-03-29 14:35 UTC (permalink / raw)
To: Han, Weidong; +Cc: 'kvm@vger.kernel.org'
Han, Weidong wrote:
>> I suggest replacing the parsing code with pci_parse_devaddr() (needs
>> to be extended to support functions) so that all the checking and
>> parsing is done in one place.
>>
>
> If use pci_parse_devaddr(), it needs to add domain section to assigning command, and add function section to pci_add/pci_del commands. What's more, pci_parse_devaddr() parses guest device bdf, there are some assumption, such as function is 0. But here parse host bdf. It's a little complex to combine them together.
>
Right, but we end up with overall better code.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 2/2] kvm: qemu: check device assignment command
2009-03-29 14:35 ` Avi Kivity
@ 2009-03-30 12:41 ` Han, Weidong
2009-04-01 9:18 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Han, Weidong @ 2009-03-30 12:41 UTC (permalink / raw)
To: 'Avi Kivity'; +Cc: 'kvm@vger.kernel.org'
Avi Kivity wrote:
> Han, Weidong wrote:
>>> I suggest replacing the parsing code with pci_parse_devaddr() (needs
>>> to be extended to support functions) so that all the checking and
>>> parsing is done in one place.
>>>
>>
>> If use pci_parse_devaddr(), it needs to add domain section to
>> assigning command, and add function section to pci_add/pci_del
>> commands. What's more, pci_parse_devaddr() parses guest device bdf,
>> there are some assumption, such as function is 0. But here parse
>> host bdf. It's a little complex to combine them together.
>>
>
> Right, but we end up with overall better code.
pci_parse_devaddr parses [[<domain>:][<bus>:]<slot>, it's valid when even enter only slot, whereas it must be bus:slot.func in device assignment command (-pcidevice host=bus:slot.func). So I implemented a dedicated function to parse device bdf in device assignment command, rather than mix two parsing function together.
Signed-off-by: Weidong Han <weidong.han@intel.com>
diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index cef7c8a..53375ff 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -1195,8 +1195,7 @@ out:
*/
AssignedDevInfo *add_assigned_device(const char *arg)
{
- char *cp, *cp1;
- char device[8];
+ char device[16];
char dma[6];
int r;
AssignedDevInfo *adev;
@@ -1207,6 +1206,13 @@ AssignedDevInfo *add_assigned_device(const char *arg)
return NULL;
}
r = get_param_value(device, sizeof(device), "host", arg);
+ if (!r)
+ goto bad;
+
+ r = pci_parse_host_devaddr(device, &adev->bus, &adev->dev, &adev->func);
+ if (r)
+ goto bad;
+
r = get_param_value(adev->name, sizeof(adev->name), "name", arg);
if (!r)
snprintf(adev->name, sizeof(adev->name), "%s", device);
@@ -1216,18 +1222,6 @@ AssignedDevInfo *add_assigned_device(const char *arg)
if (r && !strncmp(dma, "none", 4))
adev->disable_iommu = 1;
#endif
- cp = device;
- adev->bus = strtoul(cp, &cp1, 16);
- if (*cp1 != ':')
- goto bad;
- cp = cp1 + 1;
-
- adev->dev = strtoul(cp, &cp1, 16);
- if (*cp1 != '.')
- goto bad;
- cp = cp1 + 1;
-
- adev->func = strtoul(cp, &cp1, 16);
LIST_INSERT_HEAD(&adev_head, adev, next);
return adev;
diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c
index eca0517..bf97c8c 100644
--- a/qemu/hw/pci.c
+++ b/qemu/hw/pci.c
@@ -163,6 +163,7 @@ static int pci_set_default_subsystem_id(PCIDevice *pci_dev)
}
/*
+ * Parse pci address in qemu command
* Parse [[<domain>:]<bus>:]<slot>, return -1 on error
*/
static int pci_parse_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
@@ -211,6 +212,55 @@ static int pci_parse_devaddr(const char *addr, int *domp, int *busp, unsigned *s
return 0;
}
+/*
+ * Parse device bdf in device assignment command:
+ *
+ * -pcidevice host=bus:dev.func
+ *
+ * Parse <bus>:<slot>.<func> return -1 on error
+ */
+int pci_parse_host_devaddr(const char *addr, int *busp,
+ int *slotp, int *funcp)
+{
+ const char *p;
+ char *e;
+ int val;
+ int bus = 0, slot = 0, func = 0;
+
+ p = addr;
+ val = strtoul(p, &e, 16);
+ if (e == p)
+ return -1;
+ if (*e == ':') {
+ bus = val;
+ p = e + 1;
+ val = strtoul(p, &e, 16);
+ if (e == p)
+ return -1;
+ if (*e == '.') {
+ slot = val;
+ p = e + 1;
+ val = strtoul(p, &e, 16);
+ if (e == p)
+ return -1;
+ func = val;
+ } else
+ return -1;
+ } else
+ return -1;
+
+ if (bus > 0xff || slot > 0x1f || func > 0x7)
+ return -1;
+
+ if (*e)
+ return -1;
+
+ *busp = bus;
+ *slotp = slot;
+ *funcp = func;
+ return 0;
+}
+
int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
{
char devaddr[32];
diff --git a/qemu/hw/pci.h b/qemu/hw/pci.h
index a7438f2..bfdd29a 100644
--- a/qemu/hw/pci.h
+++ b/qemu/hw/pci.h
@@ -227,6 +227,9 @@ PCIDevice *pci_find_device(int bus_num, int slot, int function);
int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
+int pci_parse_host_devaddr(const char *addr, int *busp,
+ int *slotp, int *funcp);
+
void pci_info(Monitor *mon);
PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
pci_map_irq_fn map_irq, const char *name);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] kvm: qemu: check device assignment command
2009-03-30 12:41 ` Han, Weidong
@ 2009-04-01 9:18 ` Avi Kivity
0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2009-04-01 9:18 UTC (permalink / raw)
To: Han, Weidong; +Cc: 'kvm@vger.kernel.org'
Han, Weidong wrote:
> pci_parse_devaddr parses [[<domain>:][<bus>:]<slot>, it's valid when even enter only slot, whereas it must be bus:slot.func in device assignment command (-pcidevice host=bus:slot.func). So I implemented a dedicated function to parse device bdf in device assignment command, rather than mix two parsing function together.
>
>
Applied, thanks.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-04-01 9:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-26 9:02 [PATCH 2/2] kvm: qemu: check device assignment command Han, Weidong
2009-03-26 12:04 ` Avi Kivity
2009-03-27 9:31 ` Han, Weidong
2009-03-29 14:35 ` Avi Kivity
2009-03-30 12:41 ` Han, Weidong
2009-04-01 9:18 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox