pci: add accessors to get/set registers by mask

PATCH Applied Michael S. Tsirkin 3 participants 3 messages Feb 21, 2012 – Feb 22, 2012

Share & tools

Michael S. Tsirkin

author

2012-02-21 13:41 UTC #0 lore raw

PATCH 1/1 pci: add accessors to get/set registers by mask

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 <[email protected]>

---
 hw/pci.h |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 61 insertions(+), 0 deletions(-)

hw/pci.h +61 -0

index 448c44e..1103838 100644
@@ -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,

Signature · 1 line

1.7.9.111.gf3fb0

Isaku Yamahata

2012-02-22 15:58 UTC #1 lore raw

sounds good idea and the patch Looks good.

Quoted 103 lines · Michael S. Tsirkin

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 <[email protected]>

---
 hw/pci.h |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 61 insertions(+), 0 deletions(-)

hw/pci.h +61 -0

index 448c44e..1103838 100644
@@ -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,

Signature · 1 line

1.7.9.111.gf3fb0

Signature · 1 line

yamahata

Anthony Liguori

Applied

2012-02-22 18:46 UTC #2 lore raw

In reply to Michael S. Tsirkin

Quoted 19 lines · Michael S. Tsirkin

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)

Quoted 5 lines

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<[email protected]>

Applied. Thanks.

Regards,

Anthony Liguori

Quoted 76 lines

---
  hw/pci.h |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 files changed, 61 insertions(+), 0 deletions(-)

hw/pci.h +61 -0

index 448c44e..1103838 100644
@@ -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,