Re: [Qemu-devel] [PATCH v3 06/35] spapr/xive: introduce a XIVE interrupt presenter model

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4673 bytes --]

On Thu, May 03, 2018 at 05:10:48PM +0200, Cédric Le Goater wrote:
> On 05/03/2018 07:39 AM, David Gibson wrote:
> > On Thu, Apr 26, 2018 at 07:15:29PM +0200, Cédric Le Goater wrote:
> >> On 04/26/2018 11:27 AM, Cédric Le Goater wrote:
> >>> On 04/26/2018 09:11 AM, David Gibson wrote:
> >>>> On Thu, Apr 19, 2018 at 02:43:02PM +0200, Cédric Le Goater wrote:
> > [snip]
> >>>>> +static void xive_tm_os_write(void *opaque, hwaddr offset,
> >>>>> +                                   uint64_t value, unsigned size)
> >>>>> +{
> >>>>> +    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> >>>>> +    XiveNVT *nvt = XIVE_NVT(cpu->intc);
> >>>>> +    int i;
> >>>>> +
> >>>>> +    if (offset >= TM_SPC_ACK_EBB) {
> >>>>> +        xive_tm_write_special(nvt, offset, value, size);
> >>>>> +        return;
> >>>>> +    }
> >>>>> +
> >>>>> +    if (TM_RING(offset) != TM_QW1_OS) {
> >>>>
> >>>> Why have this if you have separate OS and user regions as you appear
> >>>> to do below?
> >>>
> >>> This is another problem we are trying to solve. 
> >>>
> >>> The registers a CPU can access depends on the TIMA view it is using. 
> >>> The OS TIMA view only sees the OS ring registers. The HV view sees all. 
> >>
> >> So, I gave a deeper look at the specs and I understood a little more 
> >> details of the concepts behind. You need to do frequent round-trips 
> >> to this document ...  
> >>
> >> These registers are accessible through four aligned pages, each exposing 
> >> a different view of the registers. First page (page address ending 
> >> in 0b00) gives access to the entire context and is reserved for the 
> >> ring 0 security monitor. The second (page address ending in 0b01) 
> >> is for the hypervisor, ring 1. The third (page address ending in 0b10) 
> >> is for the operating system, ring 2. The fourth (page address ending 
> >> in 0b11) is for user level, ring 3.
> >>
> >> The sPAPR machine runs at the OS privilege and therefore can only 
> >> accesses the OS and the User rings, 2 and 3. The others are for
> >> hypervisor levels.
> > 
> > Ok, that much is what I thought.  What I'm less clear on is what each
> > page looks like compared to the others.  Previously I thought each one
> > had the same registers, 
> 
> yes.
> 
> > just manipulating the corresponding ring.  
> 
> no. 
> 
> > Are you saying instead that each ring's page basically has a subset 
> > of the registers in the next most privileged page?
> 
> That's the idea. 

Ah, ok.

> The registers are defined as follow :
> 
> 	QW-0 User      
> 	QW-1 O/S      
> 	QW-2 Pool   
> 	QW-3 Physical 
> 
> and the pages :
> 
> - 0006030203180000 security monitor 
>   can access all registers 
> 
> - 0006030203190000 hv
>   can access all registers minus the secure regs
> 
> - 00060302031a0000 os
>   can access some of the OS (QW1) and User (QW0) registers
>  
> - 00060302031b0000 user
>   can access NSR reg of User (QW0) registers

I can see two reasonable ways of doing this:

A)

Have a single set of read/write functions.  These implement all the
registers but take a "privilege level" parameter which controls which
will actually work.  Those could then be wired up in one of two ways:

  A1) Single memory region.  The accessor derives the priv level from
  the relevant address bits, before masking it down to a single
  register page.  Then, as above

  A2) Multiple memory regions with the same accessor functions but
  different opaque pointer.  The accessor gets the priv level from
  its opaque pointer, then the address is just within a single ring's
  page.

B)

Separate memory regions with separate accessors.  The ring-0 accessor
implements the ring-0 registers, then calls the ring-1 accessor
function for everything else.  ring-1 calls ring-2 and so forth.

> On sPAPR, we can remap the os/user pages to some other base address 
> but we should keep the same page offset.

Sure.

> 
> 
> >> I will try to come with a better implementation of the model and
> >> make sure the ring numbers are respected. I am not sure we should 
> >> have only one memory region or four distinct ones with their
> >> own ops. There are some differences in the load/store of each view.
> > 
> > Right.  I'm not clear at this point if that's for good reasons, or
> > just because IBM's hardware designers don't seem to have gotten the
> > hang of Don't Repeat Yourself.
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]