Re: [Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable

[Isaku Yamahata <yamahata@valinux.co.jp>]

On Mon, Jan 04, 2010 at 11:55:10AM +0100, Alexander Graf wrote:
> 
> On 04.01.2010, at 11:45, Isaku Yamahata wrote:
> 
> > On Mon, Jan 04, 2010 at 04:26:46AM +0100, Alexander Graf wrote:
> >> 
> >> On 03.01.2010, at 21:50, Benjamin Herrenschmidt wrote:
> >> 
> >>> On Sun, 2010-01-03 at 21:27 +0100, Alexander Graf wrote:
> >>> 
> >>>> I think if unin_pci is the only user, it'd be better to do it hacky
> >>>> inside unin_pci.c. But if there's a chance there's another user, it'd
> >>>> be better to make it generic.
> >>>> 
> >>>> Since this is the first time I ever stumbled across type 0 and type 1
> >>>> PCI config space addresses, I simply don't know if there are any. Blue
> >>>> Swirls comment indicated that there are. Ben also sounded as if it's
> >>>> rather common to not use the x86 format. On the other hand, it looks
> >>>> like nobody in qemu needed it so far - and we're implementing ARM,
> >>>> MIPS and all other sorts of platforms.
> >>>> 
> >>>> So if anyone with knowledge in PCI could shed some light here, please
> >>>> do so.
> >>> 
> >>> My feeling is that what you're better off doing is to have the qemu core
> >>> take an abstract struct to identify a device config space location, that
> >>> consists of separate fields for:
> >>> 
> >>> - The PCI domain (which is what host bridge it hangs off since bus
> >>> numbers are not unique between domains)
> >>> 
> >>> - The bus number
> >> 
> >> Hm, I think it'd make more sense to just store a PCIBus pointer in there. We could then fetch the bus and domain id from there.
> >> 
> >> I'll write something up :-).
> >> 
> >> 
> >> Alex
> >> 
> > 
> > Does the following patch help?
> > I did only compile test though.
> 
> I sent out v2 already, which contains a complete resolution framework. It also allows for incremental cleanup of the users, as I'd rather like to see everyone use pci_host instead of hacky homegrown functions. But I don't think changing all at once is going to fly wrt reviewablity.

Agreed. Anyway that patch is just for discussion, not for commit.
If wanted, I'm willing to split it up into a series of patches or
rebase it on top of your patches. (Or throw it away)


> I'd be really glad if you could take a glimpse at my version. You're definitely more knowledgable in the PCI areas than me :-). I verified that it fixes Uninorth and x86 still works.

I'll have a look at it tomorrow.

-- 
yamahata