[Qemu-devel] Re: [PATCH v3 03/13] pci: introduce helper function pci_shift_word/long which returns shifted value.

[Isaku Yamahata]

On Wed, Sep 15, 2010 at 02:49:50PM +0200, Michael S. Tsirkin wrote:
> On Wed, Sep 15, 2010 at 02:38:16PM +0900, Isaku Yamahata wrote:
> > introduce helper function pci_shift_{word, long}() which returns
> > returns shifted word/long of given position and range.
> > They will be used later.
> > 
> > Signed-off-by: Isaku Yamahata <address@hidden>
> 
> So I think the reason you *think* you need these
> is because you set the wmask wrong: you make all
> capability readonly and then write a ton of
> custom code to tweak it. Instead,
> Make writeable registers writeable, even if
> read always returns 0.
> Then in your handler do
> if (dev->config[offset] & mask) {
>       handle bit write
>       dev->config[offset] &= ~mask;
> }
> 
> no range checks necessary.
> 
> If you need to do something on register change,
> just keep the old state in your structure,
> then
> 
>       dev->exp.a != dev->config[offset]
> tells you there was a change.

Okay, I'll give it a try.


> BTW if you need to do this to words or longs,
> not just bytes, maybe pci_set_word/pci_clear_word would be helpful?

Agreed.

thanks,

> > ---
> >  hw/pci.h |   19 +++++++++++++++++++
> >  1 files changed, 19 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/pci.h b/hw/pci.h
> > index f4ea97a..630631b 100644
> > --- a/hw/pci.h
> > +++ b/hw/pci.h
> > @@ -327,6 +327,25 @@ pci_config_set_interrupt_pin(uint8_t *pci_config, 
> > uint8_t val)
> >      pci_set_byte(&pci_config[PCI_INTERRUPT_PIN], val);
> >  }
> >  
> > +static inline uint32_t
> > +pci_shift_long(uint32_t addr, uint32_t val, uint32_t pos)
> > +{
> > +    if (addr >= pos) {
> > +        assert(addr - pos <= 32 / 8);
> > +        val <<= (addr - pos) * 8;
> > +    } else {
> > +        assert(pos - addr <= 32 / 8);
> > +        val >>= (pos - addr) * 8;
> > +    }
> > +    return val;
> > +}
> > +
> > +static inline uint16_t
> > +pci_shift_word(uint32_t addr, uint32_t val, uint32_t pos)
> > +{
> > +    return pci_shift_long(addr, val, pos);
> > +}
> > +
> >  typedef int (*pci_qdev_initfn)(PCIDevice *dev);
> >  typedef struct {
> >      DeviceInfo qdev;
> > -- 
> > 1.7.1.1
> 

-- 
yamahata