Patchwork [11/26] KVM: PPC: Make RMO a define

login
register
mail settings
Submitter Alexander Graf
Date June 25, 2010, 11:24 p.m.
Message ID <1277508314-915-12-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/57013/
State Not Applicable
Headers show

Comments

Alexander Graf - June 25, 2010, 11:24 p.m.
On PowerPC it's very normal to not support all of the physical RAM in real mode.
To check if we're matching on the shared page or not, we need to know the limits
so we can restrain ourselves to that range.

So let's make it a define instead of open-coding it. And while at it, let's also
increase it.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/include/asm/kvm_host.h |    2 ++
 arch/powerpc/kvm/book3s.c           |    4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)
Segher Boessenkool - June 26, 2010, 4:52 p.m.
> On PowerPC it's very normal to not support all of the physical RAM  
> in real mode.

Oh?  Are you referring to "real mode limit", or 32-bit  
implementations with
more than 32 address lines, or something else?

Either way, RMO is a really bad name for this, since that name is  
already
used for a similar but different concept.

Also, it seems you construct the physical address by masking out bits  
from
the effective address.  Most implementations will trap or machine  
check if
you address outside of physical address space, instead.


Segher
Alexander Graf - June 27, 2010, 9:08 a.m.
Am 26.06.2010 um 18:52 schrieb Segher Boessenkool <segher@kernel.crashing.org 
 >:

>> On PowerPC it's very normal to not support all of the physical RAM  
>> in real mode.
>
> Oh?  Are you referring to "real mode limit", or 32-bit  
> implementations with
> more than 32 address lines, or something else?

The former.

>
> Either way, RMO is a really bad name for this, since that name is  
> already
> used for a similar but different concept.

It's the same concept, no? Not all physical memory is accessible from  
real mode.

>
> Also, it seems you construct the physical address by masking out  
> bits from
> the effective address.  Most implementations will trap or machine  
> check if
> you address outside of physical address space, instead.

Well the only case where I remember to have hit a real RMO case is on  
the PS3 - that issues a data/instruction storage interrupt when  
accessing anything > 8MB in real mode.

So I'd argue this is heavily implementation specific.

Apart from that what I'm trying to cover is that on ppc64 accessing  
0xc0000000000000 in real mode gets you 0x0. Is there a better name for  
this?

Alex

>
Segher Boessenkool - June 29, 2010, 7:32 a.m.
>>> On PowerPC it's very normal to not support all of the physical  
>>> RAM in real mode.
>>
>> Oh?  Are you referring to "real mode limit", or 32-bit  
>> implementations with
>> more than 32 address lines, or something else?
>
> The former.

Okay.  In that case, the hypervisor can usually access all of physical
ram directly, unless it limits itself; and a supervisor gets either
no real mode access, or all ram, or some part -- whatever the hypervisor
likes.

If you access outside of ram, you will most likely get a machine check.
Depends on the implementation (like most things here).

>> Either way, RMO is a really bad name for this, since that name is  
>> already
>> used for a similar but different concept.
>
> It's the same concept, no? Not all physical memory is accessible  
> from real mode.

I think you're looking for "real mode limit":

The concept is called "offset real mode".

RMO ("real mode offset") is the value that is ORed to an effective  
address
to make the physical address (not added, "offset" is a pretty bad name).
RML ("real mode limit") is the value that has to be bigger than the  
effective
address or you will get an exception.
RMA ("real mode area") is the name for the range of addresses usable  
in offset
real mode.

>> Also, it seems you construct the physical address by masking out  
>> bits from
>> the effective address.  Most implementations will trap or machine  
>> check if
>> you address outside of physical address space, instead.
>
> Well the only case where I remember to have hit a real RMO case is  
> on the PS3 - that issues a data/instruction storage interrupt when  
> accessing anything > 8MB in real mode.
>
> So I'd argue this is heavily implementation specific.

It is.  So what is the behaviour you want to implement?

> Apart from that what I'm trying to cover is that on ppc64 accessing  
> 0xc0000000000000 in real mode gets you 0x0. Is there a better name  
> for this?

(You missed two zeroes).
In hypervisor real mode, the top few bits are magic.  They are used  
for e.g.
enabling hypervisor offset real mode.
In supervisor real mode, those bits are ignored (and all other bits  
that do
not correspond to physical address lines may also be ignored).


Maybe you want to call it physical_address_mask or similar?


Segher
Alexander Graf - June 29, 2010, 7:39 a.m.
On 29.06.2010, at 09:32, Segher Boessenkool wrote:

>>>> On PowerPC it's very normal to not support all of the physical RAM in real mode.
>>> 
>>> Oh?  Are you referring to "real mode limit", or 32-bit implementations with
>>> more than 32 address lines, or something else?
>> 
>> The former.
> 
> Okay.  In that case, the hypervisor can usually access all of physical
> ram directly, unless it limits itself; and a supervisor gets either
> no real mode access, or all ram, or some part -- whatever the hypervisor
> likes.
> 
> If you access outside of ram, you will most likely get a machine check.
> Depends on the implementation (like most things here).
> 
>>> Either way, RMO is a really bad name for this, since that name is already
>>> used for a similar but different concept.
>> 
>> It's the same concept, no? Not all physical memory is accessible from real mode.
> 
> I think you're looking for "real mode limit":
> 
> The concept is called "offset real mode".
> 
> RMO ("real mode offset") is the value that is ORed to an effective address
> to make the physical address (not added, "offset" is a pretty bad name).
> RML ("real mode limit") is the value that has to be bigger than the effective
> address or you will get an exception.
> RMA ("real mode area") is the name for the range of addresses usable in offset
> real mode.
> 
>>> Also, it seems you construct the physical address by masking out bits from
>>> the effective address.  Most implementations will trap or machine check if
>>> you address outside of physical address space, instead.
>> 
>> Well the only case where I remember to have hit a real RMO case is on the PS3 - that issues a data/instruction storage interrupt when accessing anything > 8MB in real mode.
>> 
>> So I'd argue this is heavily implementation specific.
> 
> It is.  So what is the behaviour you want to implement?

The one below.

> 
>> Apart from that what I'm trying to cover is that on ppc64 accessing 0xc0000000000000 in real mode gets you 0x0. Is there a better name for this?
> 
> (You missed two zeroes).
> In hypervisor real mode, the top few bits are magic.  They are used for e.g.
> enabling hypervisor offset real mode.
> In supervisor real mode, those bits are ignored (and all other bits that do
> not correspond to physical address lines may also be ignored).

So which bits exactly are reserved? I couldn't find a reference to that part.

> Maybe you want to call it physical_address_mask or similar?

PAM - doesn't sound bad :).


Alex
Segher Boessenkool - June 29, 2010, 7:52 a.m.
>>>> Also, it seems you construct the physical address by masking out  
>>>> bits from
>>>> the effective address.  Most implementations will trap or  
>>>> machine check if
>>>> you address outside of physical address space, instead.
>>>
>>> Well the only case where I remember to have hit a real RMO case  
>>> is on the PS3 - that issues a data/instruction storage interrupt  
>>> when accessing anything > 8MB in real mode.
>>>
>>> So I'd argue this is heavily implementation specific.
>>
>> It is.  So what is the behaviour you want to implement?
>
> The one below.

I'm sorry, I lost it.  "Below"?

>>> Apart from that what I'm trying to cover is that on ppc64  
>>> accessing 0xc0000000000000 in real mode gets you 0x0. Is there a  
>>> better name for this?
>>
>> (You missed two zeroes).
>> In hypervisor real mode, the top few bits are magic.  They are  
>> used for e.g.
>> enabling hypervisor offset real mode.
>> In supervisor real mode, those bits are ignored (and all other  
>> bits that do
>> not correspond to physical address lines may also be ignored).
>
> So which bits exactly are reserved? I couldn't find a reference to  
> that part.

If by "reserved" you mean "cannot be used for addressing", it's the  
top four
bits.  Book III-S chapter 5.7.3 in the Power Architecture 2.06 document.
Implementations are allowed to ignore more bits than that.

I believe in earlier versions of the architecture it was the top two  
bits,
not four, but maybe I misremember.

>> Maybe you want to call it physical_address_mask or similar?
>
> PAM - doesn't sound bad :).

And miraculously nothing in the Power arch uses that acronym yet!  But I
would spell it out if I were you, acronyms are confusing.


Segher
Alexander Graf - June 29, 2010, 8:04 a.m.
On 29.06.2010, at 09:52, Segher Boessenkool wrote:

>>>>> Also, it seems you construct the physical address by masking out bits from
>>>>> the effective address.  Most implementations will trap or machine check if
>>>>> you address outside of physical address space, instead.
>>>> 
>>>> Well the only case where I remember to have hit a real RMO case is on the PS3 - that issues a data/instruction storage interrupt when accessing anything > 8MB in real mode.
>>>> 
>>>> So I'd argue this is heavily implementation specific.
>>> 
>>> It is.  So what is the behaviour you want to implement?
>> 
>> The one below.
> 
> I'm sorry, I lost it.  "Below"?

Well, the ones a few lines below :).

> 
>>>> Apart from that what I'm trying to cover is that on ppc64 accessing 0xc0000000000000 in real mode gets you 0x0. Is there a better name for this?
>>> 
>>> (You missed two zeroes).
>>> In hypervisor real mode, the top few bits are magic.  They are used for e.g.
>>> enabling hypervisor offset real mode.
>>> In supervisor real mode, those bits are ignored (and all other bits that do
>>> not correspond to physical address lines may also be ignored).
>> 
>> So which bits exactly are reserved? I couldn't find a reference to that part.
> 
> If by "reserved" you mean "cannot be used for addressing", it's the top four
> bits.  Book III-S chapter 5.7.3 in the Power Architecture 2.06 document.
> Implementations are allowed to ignore more bits than that.
> 
> I believe in earlier versions of the architecture it was the top two bits,
> not four, but maybe I misremember.

Ah, nice. So that part is implementation specific too. Awesome ;).

> 
>>> Maybe you want to call it physical_address_mask or similar?
>> 
>> PAM - doesn't sound bad :).
> 
> And miraculously nothing in the Power arch uses that acronym yet!  But I
> would spell it out if I were you, acronyms are confusing.

Well, the bad thing about not using acronyms here is that I'll run out of the 80 character limit pretty soon. And that means line wraps and more confusingness when reading the code.

Alex

Patch

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 83c45ea..e35c1ac 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -47,6 +47,8 @@ 
 #define HPTEG_HASH_NUM_VPTE		(1 << HPTEG_HASH_BITS_VPTE)
 #define HPTEG_HASH_NUM_VPTE_LONG	(1 << HPTEG_HASH_BITS_VPTE_LONG)
 
+#define KVM_RMO			0x0fffffffffffffffULL
+
 struct kvm;
 struct kvm_run;
 struct kvm_vcpu;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index e76c950..2f55aa5 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -462,7 +462,7 @@  static int kvmppc_xlate(struct kvm_vcpu *vcpu, ulong eaddr, bool data,
 		r = vcpu->arch.mmu.xlate(vcpu, eaddr, pte, data);
 	} else {
 		pte->eaddr = eaddr;
-		pte->raddr = eaddr & 0xffffffff;
+		pte->raddr = eaddr & KVM_RMO;
 		pte->vpage = VSID_REAL | eaddr >> 12;
 		pte->may_read = true;
 		pte->may_write = true;
@@ -576,7 +576,7 @@  int kvmppc_handle_pagefault(struct kvm_run *run, struct kvm_vcpu *vcpu,
 		pte.may_execute = true;
 		pte.may_read = true;
 		pte.may_write = true;
-		pte.raddr = eaddr & 0xffffffff;
+		pte.raddr = eaddr & KVM_RMO;
 		pte.eaddr = eaddr;
 		pte.vpage = eaddr >> 12;
 	}