* [Qemu-devel] [PATCH] adding helper pci functions
@ 2010-02-25 8:41 Gerd Hoffmann
2010-02-25 10:55 ` [Qemu-devel] " Juan Quintela
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2010-02-25 8:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Izik Eidus, Gerd Hoffmann
From: Izik Eidus <ieidus@redhat.com>
Signed-off-by: Izik Eidus <ieidus@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/pci.h | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/hw/pci.h b/hw/pci.h
index 37ebdc4..20c670e 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -301,11 +301,29 @@ pci_config_set_device_id(uint8_t *pci_config, uint16_t val)
}
static inline void
+pci_config_set_revision(uint8_t *pci_config, uint8_t val)
+{
+ pci_set_byte(&pci_config[PCI_REVISION_ID], val);
+}
+
+static inline void
pci_config_set_class(uint8_t *pci_config, uint16_t val)
{
pci_set_word(&pci_config[PCI_CLASS_DEVICE], val);
}
+static inline void
+pci_config_set_prog_interface(uint8_t *pci_config, uint8_t val)
+{
+ pci_set_byte(&pci_config[PCI_CLASS_PROG], val);
+}
+
+static inline void
+pci_config_set_interrupt_pin(uint8_t *pci_config, uint8_t val)
+{
+ pci_set_byte(&pci_config[PCI_INTERRUPT_PIN], val);
+}
+
typedef int (*pci_qdev_initfn)(PCIDevice *dev);
typedef struct {
DeviceInfo qdev;
--
1.6.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH] adding helper pci functions
2010-02-25 8:41 [Qemu-devel] [PATCH] adding helper pci functions Gerd Hoffmann
@ 2010-02-25 10:55 ` Juan Quintela
2010-02-25 11:30 ` Michael S. Tsirkin
2010-02-25 11:20 ` Izik Eidus
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Juan Quintela @ 2010-02-25 10:55 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Izik Eidus, qemu-devel, Michael S. Tsirkin
Gerd Hoffmann <kraxel@redhat.com> wrote:
> From: Izik Eidus <ieidus@redhat.com>
>
> Signed-off-by: Izik Eidus <ieidus@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/pci.h | 18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.h b/hw/pci.h
> index 37ebdc4..20c670e 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -301,11 +301,29 @@ pci_config_set_device_id(uint8_t *pci_config, uint16_t val)
> }
>
> static inline void
> +pci_config_set_revision(uint8_t *pci_config, uint8_t val)
> +{
> + pci_set_byte(&pci_config[PCI_REVISION_ID], val);
> +}
> +
> +static inline void
> pci_config_set_class(uint8_t *pci_config, uint16_t val)
> {
> pci_set_word(&pci_config[PCI_CLASS_DEVICE], val);
> }
>
> +static inline void
> +pci_config_set_prog_interface(uint8_t *pci_config, uint8_t val)
> +{
> + pci_set_byte(&pci_config[PCI_CLASS_PROG], val);
> +}
> +
> +static inline void
> +pci_config_set_interrupt_pin(uint8_t *pci_config, uint8_t val)
> +{
> + pci_set_byte(&pci_config[PCI_INTERRUPT_PIN], val);
> +}
> +
> typedef int (*pci_qdev_initfn)(PCIDevice *dev);
> typedef struct {
> DeviceInfo qdev;
mst had some reservations abotu this functions, but I have forgotten.
mst can you comment again?
Later, Juan.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH] adding helper pci functions
2010-02-25 8:41 [Qemu-devel] [PATCH] adding helper pci functions Gerd Hoffmann
2010-02-25 10:55 ` [Qemu-devel] " Juan Quintela
@ 2010-02-25 11:20 ` Izik Eidus
2010-03-09 14:24 ` [Qemu-devel] " Anthony Liguori
2010-03-09 14:24 ` Anthony Liguori
3 siblings, 0 replies; 6+ messages in thread
From: Izik Eidus @ 2010-02-25 11:20 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 02/25/2010 10:41 AM, Gerd Hoffmann wrote:
> From: Izik Eidus<ieidus@redhat.com>
>
The true auther of this patch is Yaniv Kamay.
Thanks.
> Signed-off-by: Izik Eidus<ieidus@redhat.com>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PATCH] adding helper pci functions
2010-02-25 10:55 ` [Qemu-devel] " Juan Quintela
@ 2010-02-25 11:30 ` Michael S. Tsirkin
0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2010-02-25 11:30 UTC (permalink / raw)
To: Juan Quintela; +Cc: Izik Eidus, Gerd Hoffmann, qemu-devel
On Thu, Feb 25, 2010 at 11:55:25AM +0100, Juan Quintela wrote:
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> > From: Izik Eidus <ieidus@redhat.com>
> >
> > Signed-off-by: Izik Eidus <ieidus@redhat.com>
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> > hw/pci.h | 18 ++++++++++++++++++
> > 1 files changed, 18 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/pci.h b/hw/pci.h
> > index 37ebdc4..20c670e 100644
> > --- a/hw/pci.h
> > +++ b/hw/pci.h
> > @@ -301,11 +301,29 @@ pci_config_set_device_id(uint8_t *pci_config, uint16_t val)
> > }
> >
> > static inline void
> > +pci_config_set_revision(uint8_t *pci_config, uint8_t val)
> > +{
> > + pci_set_byte(&pci_config[PCI_REVISION_ID], val);
> > +}
> > +
> > +static inline void
> > pci_config_set_class(uint8_t *pci_config, uint16_t val)
> > {
> > pci_set_word(&pci_config[PCI_CLASS_DEVICE], val);
> > }
> >
> > +static inline void
> > +pci_config_set_prog_interface(uint8_t *pci_config, uint8_t val)
> > +{
> > + pci_set_byte(&pci_config[PCI_CLASS_PROG], val);
> > +}
> > +
> > +static inline void
> > +pci_config_set_interrupt_pin(uint8_t *pci_config, uint8_t val)
> > +{
> > + pci_set_byte(&pci_config[PCI_INTERRUPT_PIN], val);
> > +}
> > +
This one is wrong I think. Devices have no business touching
interrupt pin register.
> > typedef int (*pci_qdev_initfn)(PCIDevice *dev);
> > typedef struct {
> > DeviceInfo qdev;
>
> mst had some reservations abotu this functions, but I have forgotten.
>
> mst can you comment again?
>
> Later, Juan.
Yes, I commented on this internally.
By the last count, we have about 600 registers. Adding incline functions
for all of them is just a source of bugs, using pci_regs directly is
better in that sense. Even worse, this creates two legal ways to
do the same thing. So no one thing you can grep for. We could convert
all devices, but it's a lot of churn for very little benefit,
and I don't see how such a change can be tested.
So I think really only the common registers should have inline
functions, and for most devices, class/revision/prog interface
should just have default values.
Finally, the APIs as proposed here just do not get us much.
You still must remember which values to put in:
pci_config_set_prog_interface(config, PCI_CLASS_SERIAL_USB)
will happily compile, and you still must verify that
value you put in does fit in.
What I would really like to see is higher level APIs.
So pci_init_legacy_vga_adapter might make sense,
it would not just set class, but also register
IO handlers for legacy ports.
--
MST
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] adding helper pci functions
2010-02-25 8:41 [Qemu-devel] [PATCH] adding helper pci functions Gerd Hoffmann
2010-02-25 10:55 ` [Qemu-devel] " Juan Quintela
2010-02-25 11:20 ` Izik Eidus
@ 2010-03-09 14:24 ` Anthony Liguori
2010-03-09 14:24 ` Anthony Liguori
3 siblings, 0 replies; 6+ messages in thread
From: Anthony Liguori @ 2010-03-09 14:24 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Izik Eidus, qemu-devel
On 02/25/2010 02:41 AM, Gerd Hoffmann wrote:
> From: Izik Eidus<ieidus@redhat.com>
>
> Signed-off-by: Izik Eidus<ieidus@redhat.com>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
Applied. Thanks.
Regards,
Anthony Liguori
> ---
> hw/pci.h | 18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.h b/hw/pci.h
> index 37ebdc4..20c670e 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -301,11 +301,29 @@ pci_config_set_device_id(uint8_t *pci_config, uint16_t val)
> }
>
> static inline void
> +pci_config_set_revision(uint8_t *pci_config, uint8_t val)
> +{
> + pci_set_byte(&pci_config[PCI_REVISION_ID], val);
> +}
> +
> +static inline void
> pci_config_set_class(uint8_t *pci_config, uint16_t val)
> {
> pci_set_word(&pci_config[PCI_CLASS_DEVICE], val);
> }
>
> +static inline void
> +pci_config_set_prog_interface(uint8_t *pci_config, uint8_t val)
> +{
> + pci_set_byte(&pci_config[PCI_CLASS_PROG], val);
> +}
> +
> +static inline void
> +pci_config_set_interrupt_pin(uint8_t *pci_config, uint8_t val)
> +{
> + pci_set_byte(&pci_config[PCI_INTERRUPT_PIN], val);
> +}
> +
> typedef int (*pci_qdev_initfn)(PCIDevice *dev);
> typedef struct {
> DeviceInfo qdev;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] adding helper pci functions
2010-02-25 8:41 [Qemu-devel] [PATCH] adding helper pci functions Gerd Hoffmann
` (2 preceding siblings ...)
2010-03-09 14:24 ` [Qemu-devel] " Anthony Liguori
@ 2010-03-09 14:24 ` Anthony Liguori
3 siblings, 0 replies; 6+ messages in thread
From: Anthony Liguori @ 2010-03-09 14:24 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Izik Eidus, qemu-devel
On 02/25/2010 02:41 AM, Gerd Hoffmann wrote:
> From: Izik Eidus<ieidus@redhat.com>
>
> Signed-off-by: Izik Eidus<ieidus@redhat.com>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
Applied. Thanks.
Regards,
Anthony Liguori
> ---
> hw/pci.h | 18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.h b/hw/pci.h
> index 37ebdc4..20c670e 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -301,11 +301,29 @@ pci_config_set_device_id(uint8_t *pci_config, uint16_t val)
> }
>
> static inline void
> +pci_config_set_revision(uint8_t *pci_config, uint8_t val)
> +{
> + pci_set_byte(&pci_config[PCI_REVISION_ID], val);
> +}
> +
> +static inline void
> pci_config_set_class(uint8_t *pci_config, uint16_t val)
> {
> pci_set_word(&pci_config[PCI_CLASS_DEVICE], val);
> }
>
> +static inline void
> +pci_config_set_prog_interface(uint8_t *pci_config, uint8_t val)
> +{
> + pci_set_byte(&pci_config[PCI_CLASS_PROG], val);
> +}
> +
> +static inline void
> +pci_config_set_interrupt_pin(uint8_t *pci_config, uint8_t val)
> +{
> + pci_set_byte(&pci_config[PCI_INTERRUPT_PIN], val);
> +}
> +
> typedef int (*pci_qdev_initfn)(PCIDevice *dev);
> typedef struct {
> DeviceInfo qdev;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-03-09 14:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-25 8:41 [Qemu-devel] [PATCH] adding helper pci functions Gerd Hoffmann
2010-02-25 10:55 ` [Qemu-devel] " Juan Quintela
2010-02-25 11:30 ` Michael S. Tsirkin
2010-02-25 11:20 ` Izik Eidus
2010-03-09 14:24 ` [Qemu-devel] " Anthony Liguori
2010-03-09 14:24 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).