Re: [Qemu-devel] [PATCH 3/3] Check pci slot number in parse_pci_devfn
Add Cc: m...@redhat.com. MAX_PCI_SLOTS should be in pci.h instead of qdev.h? And the name should be start with PCI_ prefix for consistency? Except that, the patches look okay. thanks, On Tue, Aug 24, 2010 at 02:49:27PM +0800, Ken CC wrote: Define MAX_PCI_SLOTS as 0x1f, if pci addr provided from command line is bigger than 0x1f, return error -EINVAL. 0x1f 3 | 7 == 255 (PCIBUS_MAX_DEVICES - 1) Signed-off-by: Ken CC ken.c...@gmail.com --- hw/qdev-properties.c |2 ++ hw/qdev.h|3 +++ 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 9219cd7..96d814c 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -514,6 +514,8 @@ static int parse_pci_devfn(DeviceState *dev, Property *prop, const char *str) return -EINVAL; } } +if (slot MAX_PCI_SLOTS) +return -EINVAL; if (str[n] != '\0') return -EINVAL; if (fn 7) diff --git a/hw/qdev.h b/hw/qdev.h index 678f8b7..fcfe52e 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -7,6 +7,9 @@ #include qemu-char.h #include qemu-option.h + +#define MAX_PCI_SLOTS 0x1f + typedef struct Property Property; typedef struct PropertyInfo PropertyInfo; -- yamahata -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] Check pci slot number in parse_pci_devfn
On 08/24/2010 02:35 PM, Isaku Yamahata wrote: Add Cc: m...@redhat.com. MAX_PCI_SLOTS should be in pci.h instead of qdev.h? And the name should be start with PCI_ prefix for consistency? Except that, the patches look okay. These aren't slots, are they? They are functions. There's a lot of slot/function confusion in qemu. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] Check pci slot number in parse_pci_devfn
On Tue, Aug 24, 2010 at 02:42:18PM +0300, Avi Kivity wrote: On 08/24/2010 02:35 PM, Isaku Yamahata wrote: Add Cc: m...@redhat.com. MAX_PCI_SLOTS should be in pci.h instead of qdev.h? And the name should be start with PCI_ prefix for consistency? Except that, the patches look okay. These aren't slots, are they? They are functions. The function checks if given $slot.$fn (or $slot) is valid. So it's slots. max 32. There's a lot of slot/function confusion in qemu. Agreed. -- yamahata -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] Check pci slot number in parse_pci_devfn
On 08/24/2010 03:07 PM, Isaku Yamahata wrote: On Tue, Aug 24, 2010 at 02:42:18PM +0300, Avi Kivity wrote: On 08/24/2010 02:35 PM, Isaku Yamahata wrote: Add Cc: m...@redhat.com. MAX_PCI_SLOTS should be in pci.h instead of qdev.h? And the name should be start with PCI_ prefix for consistency? Except that, the patches look okay. These aren't slots, are they? They are functions. The function checks if given $slot.$fn (or $slot) is valid. So it's slots. max 32. +assert(devfn PCIBUS_MAX_DEVICES); Looks like we're comparing a function number to PCIBUS_MAX_DEVICES. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] Check pci slot number in parse_pci_devfn
On Tue, Aug 24, 2010 at 03:04:44PM +0300, Avi Kivity wrote: On 08/24/2010 03:07 PM, Isaku Yamahata wrote: On Tue, Aug 24, 2010 at 02:42:18PM +0300, Avi Kivity wrote: On 08/24/2010 02:35 PM, Isaku Yamahata wrote: Add Cc: m...@redhat.com. MAX_PCI_SLOTS should be in pci.h instead of qdev.h? And the name should be start with PCI_ prefix for consistency? Except that, the patches look okay. These aren't slots, are they? They are functions. The function checks if given $slot.$fn (or $slot) is valid. So it's slots. max 32. +assert(devfn PCIBUS_MAX_DEVICES); Looks like we're comparing a function number to PCIBUS_MAX_DEVICES. Ah, you're talking about 2/3. I talked about 3/3. You're right. The name is misleading. PCIBUS_MAX_FUNCTIONS? Or suggestions? -- yamahata -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] Check pci slot number in parse_pci_devfn
On 08/24/2010 03:24 PM, Isaku Yamahata wrote: On Tue, Aug 24, 2010 at 03:04:44PM +0300, Avi Kivity wrote: On 08/24/2010 03:07 PM, Isaku Yamahata wrote: On Tue, Aug 24, 2010 at 02:42:18PM +0300, Avi Kivity wrote: On 08/24/2010 02:35 PM, Isaku Yamahata wrote: Add Cc: m...@redhat.com. MAX_PCI_SLOTS should be in pci.h instead of qdev.h? And the name should be start with PCI_ prefix for consistency? Except that, the patches look okay. These aren't slots, are they? They are functions. The function checks if given $slot.$fn (or $slot) is valid. So it's slots. max 32. +assert(devfn PCIBUS_MAX_DEVICES); Looks like we're comparing a function number to PCIBUS_MAX_DEVICES. Ah, you're talking about 2/3. I talked about 3/3. You're right. The name is misleading. PCIBUS_MAX_FUNCTIONS? Or suggestions? Or assert(devfn / 8 PCIBUS_MAX_DEVICES) (and change that to be 32). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] Check pci slot number in parse_pci_devfn
On Tue, Aug 24, 2010 at 03:04:44PM +0300, Avi Kivity wrote: On 08/24/2010 03:07 PM, Isaku Yamahata wrote: On Tue, Aug 24, 2010 at 02:42:18PM +0300, Avi Kivity wrote: On 08/24/2010 02:35 PM, Isaku Yamahata wrote: Add Cc: m...@redhat.com. MAX_PCI_SLOTS should be in pci.h instead of qdev.h? And the name should be start with PCI_ prefix for consistency? Except that, the patches look okay. These aren't slots, are they? They are functions. The function checks if given $slot.$fn (or $slot) is valid. So it's slots. max 32. +assert(devfn PCIBUS_MAX_DEVICES); Looks like we're comparing a function number to PCIBUS_MAX_DEVICES. PCIBUS_MAX_DEVICES is the size of PCIBus.devices[], I have added it in the first patch at the defination of struct PCIBus, line 50 hw/pci.c. so i think the better name of the macro should be PCIBUS_MAX_FN, right? Ken -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] Check pci slot number in parse_pci_devfn
On 08/24/2010 03:16 PM, Chen Cao wrote: On Tue, Aug 24, 2010 at 03:04:44PM +0300, Avi Kivity wrote: On 08/24/2010 03:07 PM, Isaku Yamahata wrote: On Tue, Aug 24, 2010 at 02:42:18PM +0300, Avi Kivity wrote: On 08/24/2010 02:35 PM, Isaku Yamahata wrote: Add Cc: m...@redhat.com. MAX_PCI_SLOTS should be in pci.h instead of qdev.h? And the name should be start with PCI_ prefix for consistency? Except that, the patches look okay. These aren't slots, are they? They are functions. The function checks if given $slot.$fn (or $slot) is valid. So it's slots. max 32. +assert(devfn PCIBUS_MAX_DEVICES); Looks like we're comparing a function number to PCIBUS_MAX_DEVICES. PCIBUS_MAX_DEVICES is the size of PCIBus.devices[], I have added it in the first patch at the defination of struct PCIBus, line 50 hw/pci.c. so i think the better name of the macro should be PCIBUS_MAX_FN, right? Or make it 32 and scale it by PCI_FUNCTIONS_PER_DEVICE. PCIBus.devices[] should be renamed to functions[] (later). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
