Message ID | 20120221134128.GA7930@redhat.com |
---|---|
State | New |
Headers | show |
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 >
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,
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,
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(-)