Re: [PATCH v2 08/13] vt82c686: Move creation of ISA devices to the ISA bridge

[BALATON Zoltan]

On Mon, 11 Jan 2021, Jiaxun Yang wrote:
> On Mon, Jan 11, 2021, at 3:25 AM, BALATON Zoltan wrote:
> > On Sun, 10 Jan 2021, Philippe Mathieu-Daudé wrote:
> > > +PCI experts
> > >
> > > On 1/10/21 1:43 AM, BALATON Zoltan wrote:
> > > > On Sun, 10 Jan 2021, Philippe Mathieu-Daudé wrote:
>
> [...]
>
> > > I'm not a PCI expert but my understanding is PCI device functions are
> > > restricted to the PCI bus address space. The host bridge may map this
> > > space within the host.
> > >
> > > QEMU might be using get_system_memory() because for some host bridge
> > > the mapping is not implemented so it was easier this way?
> >
> > Maybe, also one less indirection which if not really needed is a good
> > thing for performance so unless it's found to be needed to use another
> > address space here I'm happy with this as it matches what other similar
> > devices do and it seems to work. Maybe a separate address space is only
> > really needed if we have an iommu?
>
> Hi Zoltan,
>
> It is possible for bonito to remap PCI address space so maybe it's essential 
> for bonito.

I'm still not sure it's needed or how that would work. Maybe while it's 
possible to remap these, all guests just map these one-to-one (otherwise 
they may need to do some address translation which is much better avoided) 
so in practice we don't need to emulate this. If we use a different memory 
region maybe we also need some additional iommu code somewhere to connect 
the two but I'm not sure, I haven't tried since most other isa bridges do 
the same way using system_memory and this seems to work. I'm also a bit 
concerned about additional overhead which we could avoid if possible. Just 
to model something that's not really used I don't think it's worth the 
additional complexity and possible performance loss unless it's found to 
be needed to get some guests work.

> Appreciate for your work. I'm going to help with reviewing as well.

Thanks, I hope to get these in now before the freeze so testing and 
reviewing is really appreciated.

Regards,
BALATON Zoltan