On Sun, Jan 03, 2010 at 05:13:12PM +0100, Alexander Graf wrote:
>
> On 03.01.2010, at 16:47, Michael S. Tsirkin wrote:
>
> > On Sun, Jan 03, 2010 at 04:40:12PM +0100, Alexander Graf wrote:
> >>
> >> On 03.01.2010, at 16:32, Michael S. Tsirkin wrote:
> >>
> >>> On Sun, Jan 03, 2010 at 02:50:46AM +0100, Alexander Graf wrote:
> >>>> As stated in the previous patch, the Uninorth PCI bridge requires
> >>>> different
> >>>> layouts in its PCI config space accessors.
> >>>>
> >>>> This patch introduces a conversion function that makes it compatible with
> >>>> the way Linux accesses it.
> >>>>
> >>>> I also kept an OpenBIOS compatibility hack in. I think it'd be better to
> >>>> take small steps here and do the config space access rework in OpenBIOS
> >>>> later on. When that's done we can remove that hack.
> >>>>
> >>>> Signed-off-by: Alexander Graf <ag...@suse.de>
> >>>> ---
> >>>> hw/unin_pci.c | 35 +++++++++++++++++++++++++++++++++++
> >>>> 1 files changed, 35 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> >>>> index fdb9401..1c49008 100644
> >>>> --- a/hw/unin_pci.c
> >>>> +++ b/hw/unin_pci.c
> >>>> @@ -75,6 +75,40 @@ static void pci_unin_reset(void *opaque)
> >>>> {
> >>>> }
> >>>>
> >>>> +static uint32_t unin_get_config_reg(PCIHostState *s, uint32_t addr)
> >>>> +{
> >>>> + uint32_t retval;
> >>>> + uint32_t reg = s->config_reg;
> >>>> +
> >>>> + if (reg & (1u << 31)) {
> >>>> + /* XXX OpenBIOS compatibility hack */
> >>>> + retval = reg;
> >>>> + addr |= reg & 7;
> >>>> + } else if (reg & 1) {
> >>>> + /* Set upper valid bit and remove lower one */
> >>>> + retval = (reg & ~3u) | (1u << 31);
> >>>> + } else {
> >>>> + uint32_t slot, func;
> >>>> + uint32_t devfn;
> >>>> +
> >>>> + /* Grab CFA0 style values */
> >>>> + slot = ffs(reg & 0xfffff800) - 1;
> >>>> + func = (reg >> 8) & 7;
> >>>> + devfn = PCI_DEVFN(slot, func);
> >>>> +
> >>>> + /* ... and then convert them to x86 format */
> >>>> + retval = (reg & 0xfc) | (devfn << 8) | (1u << 31);
> >>>
> >>> Is it a good idea to have a helper that encodes reg/dev/fn into a 32 bit
> >>> number? This way this encoding can be changed down the road.
> >>
> >> I don't think I understand this comment? :-)
> >
> > This puts reg+dev+fn in a format that pci_host can the understand
> > correct? So it would make sense to have an inline function
> > in pci host that gets 3 parameters and does the encoding.
>
> We're doing the reverse here. We get a uint32_t (host->config_reg) and need
> to do something with it.
>
> We could either call a helper that splits it into bus,dev,fn or we could just
> put all of them into a single uint32_t again that later on gets interpreted
> in a specified format.
>
> I figured I'd have to touch less code and keep things more stable for the
> other (non-uninorth) buses if I keep the x86 format as "default format" and
> just convert to it. Passing a uint32_t is also easier than passing 3 ints :-).
>
> Alex
So what the comment above suggests, is adding helper routine
that gets register device, function and creates 32 bit value
in "default format".
--
MST
