qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] pci: add accessors to get/set registers by mask
@ 2012-02-21 13:41 Michael S. Tsirkin
  2012-02-22 15:58 ` Isaku Yamahata
  2012-02-22 18:46 ` Anthony Liguori
  0 siblings, 2 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2012-02-21 13:41 UTC (permalink / raw)
  To: yamahata; +Cc: qemu-devel

pci_regs.h specifies many registers by mask +
shifted register values.
There's always some duplication when using such:
for example to override device type, we would need:

pci_word_test_and_clear_mask(cap + PCI_EXP_FLAGS,
     PCI_EXP_FLAGS_TYPE);
pci_word_test_and_set_mask(cap + PCI_EXP_FLAGS,
    PCI_EXP_TYPE_ENDPOINT << (ffs(PCI_EXP_FLAGS_TYPE) - 1));

Getting such registers also uses some duplication:

word = pci_get_word(cap + PCI_EXP_FLAGS) & PCI_EXP_FLAGS_TYPE;
if ((word >> ffs((PCI_EXP_FLAGS_TYPE) - 1)) == PCI_EXP_TYPE_ENDPOINT)

Add API to access such registers in one line:
pci_set_word_by_mask(cap + PCI_EXP_FLAGS, PCI_EXP_FLAGS_TYPE,
		     PCI_EXP_TYPE_ENDPOINT)

and
word = pci_get_word_by_mask(cap + PCI_EXP_FLAGS, PCI_EXP_FLAGS_TYPE)
if (word == PCI_EXP_TYPE_ENDPOINT)

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci.h |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/hw/pci.h b/hw/pci.h
index 448c44e..1103838 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -469,6 +469,67 @@ pci_quad_test_and_set_mask(uint8_t *config, uint64_t mask)
     return val & mask;
 }
 
+/* Access a register specified by a mask */
+static inline void
+pci_set_byte_by_mask(uint8_t *config, uint8_t mask, uint8_t reg)
+{
+    uint8_t val = pci_get_byte(config);
+    uint8_t rval = reg << (ffs(mask) - 1);
+    pci_set_byte(config, (~mask & val) | (mask & rval));
+}
+
+static inline uint8_t
+pci_get_byte_by_mask(uint8_t *config, uint8_t mask)
+{
+    uint8_t val = pci_get_byte(config);
+    return (val & mask) >> (ffs(mask) - 1);
+}
+
+static inline void
+pci_set_word_by_mask(uint8_t *config, uint16_t mask, uint16_t reg)
+{
+    uint16_t val = pci_get_word(config);
+    uint16_t rval = reg << (ffs(mask) - 1);
+    pci_set_word(config, (~mask & val) | (mask & rval));
+}
+
+static inline uint16_t
+pci_get_word_by_mask(uint8_t *config, uint16_t mask)
+{
+    uint16_t val = pci_get_word(config);
+    return (val & mask) >> (ffs(mask) - 1);
+}
+
+static inline void
+pci_set_long_by_mask(uint8_t *config, uint32_t mask, uint32_t reg)
+{
+    uint32_t val = pci_get_long(config);
+    uint32_t rval = reg << (ffs(mask) - 1);
+    pci_set_long(config, (~mask & val) | (mask & rval));
+}
+
+static inline uint32_t
+pci_get_long_by_mask(uint8_t *config, uint32_t mask)
+{
+    uint32_t val = pci_get_long(config);
+    return (val & mask) >> (ffs(mask) - 1);
+}
+
+static inline void
+pci_set_quad_by_mask(uint8_t *config, uint64_t mask, uint64_t reg)
+{
+    uint64_t val = pci_get_quad(config);
+    uint64_t rval = reg << (ffs(mask) - 1);
+    pci_set_quad(config, (~mask & val) | (mask & rval));
+}
+
+static inline uint64_t
+pci_get_quad_by_mask(uint8_t *config, uint64_t mask)
+{
+    uint64_t val = pci_get_quad(config);
+    return (val & mask) >> (ffs(mask) - 1);
+}
+
 PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
                                     const char *name);
 PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
-- 
1.7.9.111.gf3fb0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] pci: add accessors to get/set registers by mask
  2012-02-21 13:41 [Qemu-devel] [PATCH] pci: add accessors to get/set registers by mask Michael S. Tsirkin
@ 2012-02-22 15:58 ` Isaku Yamahata
  2012-02-22 18:46 ` Anthony Liguori
  1 sibling, 0 replies; 3+ messages in thread
From: Isaku Yamahata @ 2012-02-22 15:58 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel

sounds good idea and the patch Looks good.

On Tue, Feb 21, 2012 at 03:41:30PM +0200, Michael S. Tsirkin wrote:
> pci_regs.h specifies many registers by mask +
> shifted register values.
> There's always some duplication when using such:
> for example to override device type, we would need:
> 
> pci_word_test_and_clear_mask(cap + PCI_EXP_FLAGS,
>      PCI_EXP_FLAGS_TYPE);
> pci_word_test_and_set_mask(cap + PCI_EXP_FLAGS,
>     PCI_EXP_TYPE_ENDPOINT << (ffs(PCI_EXP_FLAGS_TYPE) - 1));
> 
> Getting such registers also uses some duplication:
> 
> word = pci_get_word(cap + PCI_EXP_FLAGS) & PCI_EXP_FLAGS_TYPE;
> if ((word >> ffs((PCI_EXP_FLAGS_TYPE) - 1)) == PCI_EXP_TYPE_ENDPOINT)
> 
> Add API to access such registers in one line:
> pci_set_word_by_mask(cap + PCI_EXP_FLAGS, PCI_EXP_FLAGS_TYPE,
> 		     PCI_EXP_TYPE_ENDPOINT)
> 
> and
> word = pci_get_word_by_mask(cap + PCI_EXP_FLAGS, PCI_EXP_FLAGS_TYPE)
> if (word == PCI_EXP_TYPE_ENDPOINT)
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/pci.h |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 61 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/pci.h b/hw/pci.h
> index 448c44e..1103838 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -469,6 +469,67 @@ pci_quad_test_and_set_mask(uint8_t *config, uint64_t mask)
>      return val & mask;
>  }
>  
> +/* Access a register specified by a mask */
> +static inline void
> +pci_set_byte_by_mask(uint8_t *config, uint8_t mask, uint8_t reg)
> +{
> +    uint8_t val = pci_get_byte(config);
> +    uint8_t rval = reg << (ffs(mask) - 1);
> +    pci_set_byte(config, (~mask & val) | (mask & rval));
> +}
> +
> +static inline uint8_t
> +pci_get_byte_by_mask(uint8_t *config, uint8_t mask)
> +{
> +    uint8_t val = pci_get_byte(config);
> +    return (val & mask) >> (ffs(mask) - 1);
> +}
> +
> +static inline void
> +pci_set_word_by_mask(uint8_t *config, uint16_t mask, uint16_t reg)
> +{
> +    uint16_t val = pci_get_word(config);
> +    uint16_t rval = reg << (ffs(mask) - 1);
> +    pci_set_word(config, (~mask & val) | (mask & rval));
> +}
> +
> +static inline uint16_t
> +pci_get_word_by_mask(uint8_t *config, uint16_t mask)
> +{
> +    uint16_t val = pci_get_word(config);
> +    return (val & mask) >> (ffs(mask) - 1);
> +}
> +
> +static inline void
> +pci_set_long_by_mask(uint8_t *config, uint32_t mask, uint32_t reg)
> +{
> +    uint32_t val = pci_get_long(config);
> +    uint32_t rval = reg << (ffs(mask) - 1);
> +    pci_set_long(config, (~mask & val) | (mask & rval));
> +}
> +
> +static inline uint32_t
> +pci_get_long_by_mask(uint8_t *config, uint32_t mask)
> +{
> +    uint32_t val = pci_get_long(config);
> +    return (val & mask) >> (ffs(mask) - 1);
> +}
> +
> +static inline void
> +pci_set_quad_by_mask(uint8_t *config, uint64_t mask, uint64_t reg)
> +{
> +    uint64_t val = pci_get_quad(config);
> +    uint64_t rval = reg << (ffs(mask) - 1);
> +    pci_set_quad(config, (~mask & val) | (mask & rval));
> +}
> +
> +static inline uint64_t
> +pci_get_quad_by_mask(uint8_t *config, uint64_t mask)
> +{
> +    uint64_t val = pci_get_quad(config);
> +    return (val & mask) >> (ffs(mask) - 1);
> +}
> +
>  PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
>                                      const char *name);
>  PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
> -- 
> 1.7.9.111.gf3fb0
> 

-- 
yamahata

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] pci: add accessors to get/set registers by mask
  2012-02-21 13:41 [Qemu-devel] [PATCH] pci: add accessors to get/set registers by mask Michael S. Tsirkin
  2012-02-22 15:58 ` Isaku Yamahata
@ 2012-02-22 18:46 ` Anthony Liguori
  1 sibling, 0 replies; 3+ messages in thread
From: Anthony Liguori @ 2012-02-22 18:46 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: yamahata, qemu-devel

On 02/21/2012 07:41 AM, Michael S. Tsirkin wrote:
> pci_regs.h specifies many registers by mask +
> shifted register values.
> There's always some duplication when using such:
> for example to override device type, we would need:
>
> pci_word_test_and_clear_mask(cap + PCI_EXP_FLAGS,
>       PCI_EXP_FLAGS_TYPE);
> pci_word_test_and_set_mask(cap + PCI_EXP_FLAGS,
>      PCI_EXP_TYPE_ENDPOINT<<  (ffs(PCI_EXP_FLAGS_TYPE) - 1));
>
> Getting such registers also uses some duplication:
>
> word = pci_get_word(cap + PCI_EXP_FLAGS)&  PCI_EXP_FLAGS_TYPE;
> if ((word>>  ffs((PCI_EXP_FLAGS_TYPE) - 1)) == PCI_EXP_TYPE_ENDPOINT)
>
> Add API to access such registers in one line:
> pci_set_word_by_mask(cap + PCI_EXP_FLAGS, PCI_EXP_FLAGS_TYPE,
> 		     PCI_EXP_TYPE_ENDPOINT)
>
> and
> word = pci_get_word_by_mask(cap + PCI_EXP_FLAGS, PCI_EXP_FLAGS_TYPE)
> if (word == PCI_EXP_TYPE_ENDPOINT)
>
> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
>   hw/pci.h |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 61 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.h b/hw/pci.h
> index 448c44e..1103838 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -469,6 +469,67 @@ pci_quad_test_and_set_mask(uint8_t *config, uint64_t mask)
>       return val&  mask;
>   }
>
> +/* Access a register specified by a mask */
> +static inline void
> +pci_set_byte_by_mask(uint8_t *config, uint8_t mask, uint8_t reg)
> +{
> +    uint8_t val = pci_get_byte(config);
> +    uint8_t rval = reg<<  (ffs(mask) - 1);
> +    pci_set_byte(config, (~mask&  val) | (mask&  rval));
> +}
> +
> +static inline uint8_t
> +pci_get_byte_by_mask(uint8_t *config, uint8_t mask)
> +{
> +    uint8_t val = pci_get_byte(config);
> +    return (val&  mask)>>  (ffs(mask) - 1);
> +}
> +
> +static inline void
> +pci_set_word_by_mask(uint8_t *config, uint16_t mask, uint16_t reg)
> +{
> +    uint16_t val = pci_get_word(config);
> +    uint16_t rval = reg<<  (ffs(mask) - 1);
> +    pci_set_word(config, (~mask&  val) | (mask&  rval));
> +}
> +
> +static inline uint16_t
> +pci_get_word_by_mask(uint8_t *config, uint16_t mask)
> +{
> +    uint16_t val = pci_get_word(config);
> +    return (val&  mask)>>  (ffs(mask) - 1);
> +}
> +
> +static inline void
> +pci_set_long_by_mask(uint8_t *config, uint32_t mask, uint32_t reg)
> +{
> +    uint32_t val = pci_get_long(config);
> +    uint32_t rval = reg<<  (ffs(mask) - 1);
> +    pci_set_long(config, (~mask&  val) | (mask&  rval));
> +}
> +
> +static inline uint32_t
> +pci_get_long_by_mask(uint8_t *config, uint32_t mask)
> +{
> +    uint32_t val = pci_get_long(config);
> +    return (val&  mask)>>  (ffs(mask) - 1);
> +}
> +
> +static inline void
> +pci_set_quad_by_mask(uint8_t *config, uint64_t mask, uint64_t reg)
> +{
> +    uint64_t val = pci_get_quad(config);
> +    uint64_t rval = reg<<  (ffs(mask) - 1);
> +    pci_set_quad(config, (~mask&  val) | (mask&  rval));
> +}
> +
> +static inline uint64_t
> +pci_get_quad_by_mask(uint8_t *config, uint64_t mask)
> +{
> +    uint64_t val = pci_get_quad(config);
> +    return (val&  mask)>>  (ffs(mask) - 1);
> +}
> +
>   PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
>                                       const char *name);
>   PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-02-22 18:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-21 13:41 [Qemu-devel] [PATCH] pci: add accessors to get/set registers by mask Michael S. Tsirkin
2012-02-22 15:58 ` Isaku Yamahata
2012-02-22 18:46 ` 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).