Re: [Qemu-arm] [PATCH v5 1/2] hw/arm: Add arm SBSA reference machine, skeleton part

[Hongbo Zhang]

On Thu, 31 Jan 2019 at 20:02, Peter Maydell <address@hidden> wrote:
>
> On Wed, 30 Jan 2019 at 08:59, Hongbo Zhang <address@hidden> wrote:
> >
> > On Tue, 22 Jan 2019 at 19:42, Peter Maydell <address@hidden> wrote:
> > >
> > > On Fri, 7 Dec 2018 at 09:08, Hongbo Zhang <address@hidden> wrote:
> > > > +    [VIRT_SECURE_MEM] =         { 0x20000000, 0x20000000 },
> > > > +    [VIRT_CPUPERIPHS] =         { 0x40000000, 0x00080000 },
> > > > +    /* GIC distributor and CPU interface expansion spaces reserved */
> > > > +    [VIRT_GIC_DIST] =           { 0x40000000, 0x00010000 },
> > > > +    [VIRT_GIC_CPU] =            { 0x40040000, 0x00010000 },
> > >
> > > If they're just reserved you don't really need to list them here,
> > > as they're covered by the VIRT_CPUPERIPHS space anyway. (You
> > > don't list the VIRT_GIC_HYP registers or VIRT_GIC_VCPU.)
> > >
> > We need more consideration here.
>
> Yeah, this is a little more complex than I thought.
>
> > Why CPUPERIPHS is used to cover DIST and CPU interface? is this from
> > some old platform? I don't see the term in GICv3 memory map in the
> > user manual, so do we still need it for this Arm64 GICv3?
> > If we only list CPUPERIPHS but without DIST, I am afraid firmware or
> > driver developer still looking for the term of DIST for base address.
>
> OK, so what CPUPERIPHS does (in the virt board) is set the
> CPU property which defines the reset value of the CBAR_EL1 register.
> (The size specified isn't used.)
>
> The set of memory mapped things in this area should (in theory)
> be the same as what the hardware CPU puts there, because guest
> code can reasonably look at CBAR and expect to find there the
> devices that the real CPU puts there. Unfortunately on the virt
> board we don't get this right (and in any case we support multiple
> CPU types which don't necessarily have the same set of devices).
> Things work in practice because Linux and OVMF both look at
> the device tree rather than assuming they can find things
> via CBAR_EL1. This is awkward to fix in the virt board without
> breaking compatibility, but we can get it right for this new board.
>
> > For GICv3, what we can have are DIST, CPU, REDIST, VCPU and HYP.
> > CPU, VCPU and HYP are not simulated for GICv3 (curious it still works
> > without CPU interface emulated), so we only have DIST and REDIST.
> > Can we only list the DIST and REDIST without CPUPERIPHS?
>
> QEMU's GICv3 implementation only supports the system register
> interface, not the memory-mapped interface. This is why we don't
> have a memory mapped thing to put into VIRT_GIC_HYP/VIRT_GIC_CPU/
> VIRT_GIC_VCPU.
>
> For the Cortex-A53/A57/A72, the only things in the CBAR area are
> these memory mapped interfaces that we don't implement:
>   CBAR+0x00000 : cpu interface
>   CBAR+0x10000 : virt interface control
>   CBAR+0x20000 : vcpu interface
>   CBAR+0x2F000 : alias of vcpu interface
> (other parts of the CBAR+0x00000 to CBAR+0x3ffff reserved)
>
> So for the SBSA reference board, I think the right thing is to
> leave a gap in the memory map of 0x40000 in size for CPUPERIPHS,
> with a comment that this is reserved CPU peripheral space
> because we don't implement the GICv3 memory-mapped CPU interface.
> Put GIC_DIST somewhere else (it is a GICv3 device register
> bank, not part of the CPU's peripherals), and don't define
> GIC_CPU at all (since you don't use it).
>
Yes, should like this, thanks.

> thanks
> -- PMM