* [Qemu-devel] [PATCH] Correction of the TLB handling of the OpenRISC target
@ 2013-10-02 5:12 Sebastian Macke
2013-10-02 5:33 ` [Qemu-devel] [OpenRISC] " Jia Liu
0 siblings, 1 reply; 5+ messages in thread
From: Sebastian Macke @ 2013-10-02 5:12 UTC (permalink / raw)
To: qemu-devel; +Cc: openrisc, openrisc
[-- Attachment #1: Type: text/plain, Size: 368 bytes --]
Hi,
this patch corrects two problems for the OpenRISC Target in QEMU. The
first one corrects one obvious bug
concerning the handling of page faults while reading from a page. The
second part removes a non-conforming behavior for the first page of the
memory.
I have tested this patch with the newest Linux kernel and compared the
output with or1ksim.
Sebastian
[-- Attachment #2: 0001-Correction-of-the-TLB-handling-of-the-OpenRISC-targe.patch --]
[-- Type: text/plain, Size: 1561 bytes --]
>From 4491bae7109e2b4de5a8de8a7e4b08d1f19ac70e Mon Sep 17 00:00:00 2001
From: Sebastian Macke <sebastian@macke.de>
Date: Tue, 1 Oct 2013 21:39:38 -0700
Subject: [PATCH] Correction of the TLB handling of the OpenRISC target
This patch correct two problems. The first one corrects one obvious bug
concerning the handling of page faults while reading from a page.
The second part removes a non-conforming behavior for the first page of
the memory.
I have tested this patch with the newest Linux kernel and compared the
output with or1ksim.
Signed-off-by: Sebastian Macke <sebastian@macke.de>
---
target-openrisc/mmu.c | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/target-openrisc/mmu.c b/target-openrisc/mmu.c
index 57f5616..22d7cbe 100644
--- a/target-openrisc/mmu.c
+++ b/target-openrisc/mmu.c
@@ -102,7 +102,7 @@ int cpu_openrisc_get_phys_data(OpenRISCCPU *cpu,
}
}
- if ((rw & 0) && ((right & PAGE_READ) == 0)) {
+ if (!(rw & 1) && ((right & PAGE_READ) == 0)) {
return TLBRET_BADADDR;
}
if ((rw & 1) && ((right & PAGE_WRITE) == 0)) {
@@ -122,13 +122,6 @@ static int cpu_openrisc_get_phys_addr(OpenRISCCPU *cpu,
{
int ret = TLBRET_MATCH;
- /* [0x0000--0x2000]: unmapped */
- if (address < 0x2000 && (cpu->env.sr & SR_SM)) {
- *physical = address;
- *prot = PAGE_READ | PAGE_WRITE;
- return ret;
- }
-
if (rw == 2) { /* ITLB */
*physical = 0;
ret = cpu->env.tlb->cpu_openrisc_map_address_code(cpu, physical,
--
1.7.9
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [OpenRISC] [PATCH] Correction of the TLB handling of the OpenRISC target
2013-10-02 5:12 [Qemu-devel] [PATCH] Correction of the TLB handling of the OpenRISC target Sebastian Macke
@ 2013-10-02 5:33 ` Jia Liu
2013-10-02 6:15 ` Stefan Kristiansson
2013-10-02 6:31 ` Sebastian Macke
0 siblings, 2 replies; 5+ messages in thread
From: Jia Liu @ 2013-10-02 5:33 UTC (permalink / raw)
To: Sebastian Macke; +Cc: openrisc, openrisc, qemu-devel@nongnu.org
Hi Sebastian,
On Wed, Oct 2, 2013 at 1:12 PM, Sebastian Macke <sebastian@macke.de> wrote:
> Hi,
>
> this patch corrects two problems for the OpenRISC Target in QEMU. The first
> one corrects one obvious bug
> concerning the handling of page faults while reading from a page.
@@ -102,7 +102,7 @@ int cpu_openrisc_get_phys_data(OpenRISCCPU *cpu,
}
}
- if ((rw & 0) && ((right & PAGE_READ) == 0)) {
+ if (!(rw & 1) && ((right & PAGE_READ) == 0)) {
return TLBRET_BADADDR;
}
if ((rw & 1) && ((right & PAGE_WRITE) == 0)) {
They are just two type of one code...
> The second
> part removes a non-conforming behavior for the first page of the memory.
@@ -122,13 +122,6 @@ static int cpu_openrisc_get_phys_addr(OpenRISCCPU *cpu,
{
int ret = TLBRET_MATCH;
- /* [0x0000--0x2000]: unmapped */
- if (address < 0x2000 && (cpu->env.sr & SR_SM)) {
- *physical = address;
- *prot = PAGE_READ | PAGE_WRITE;
- return ret;
- }
-
May you please explain more about why the first page is non-conforming?
The Arch manual told me 0x0000--0x2000 is unmapped.
>
> I have tested this patch with the newest Linux kernel and compared the
> output with or1ksim.
May you please upload a newest Linux kernel to somewhere?
>
> Sebastian
>
> _______________________________________________
> OpenRISC mailing list
> OpenRISC@lists.openrisc.net
> http://lists.openrisc.net/listinfo/openrisc
>
Regards,
Jia
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [OpenRISC] [PATCH] Correction of the TLB handling of the OpenRISC target
2013-10-02 5:33 ` [Qemu-devel] [OpenRISC] " Jia Liu
@ 2013-10-02 6:15 ` Stefan Kristiansson
2013-10-03 1:42 ` Jia Liu
2013-10-02 6:31 ` Sebastian Macke
1 sibling, 1 reply; 5+ messages in thread
From: Stefan Kristiansson @ 2013-10-02 6:15 UTC (permalink / raw)
To: Jia Liu
Cc: Sebastian Macke, openrisc, openrisc@lists.opencores.org,
qemu-devel@nongnu.org
[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]
On Wed, Oct 2, 2013 at 8:33 AM, Jia Liu <proljc@gmail.com> wrote:
> Hi Sebastian,
>
> On Wed, Oct 2, 2013 at 1:12 PM, Sebastian Macke <sebastian@macke.de>
> wrote:
> > Hi,
> >
> > this patch corrects two problems for the OpenRISC Target in QEMU. The
> first
> > one corrects one obvious bug
> > concerning the handling of page faults while reading from a page.
>
> @@ -102,7 +102,7 @@ int cpu_openrisc_get_phys_data(OpenRISCCPU *cpu,
> }
> }
>
> - if ((rw & 0) && ((right & PAGE_READ) == 0)) {
> + if (!(rw & 1) && ((right & PAGE_READ) == 0)) {
> return TLBRET_BADADDR;
> }
> if ((rw & 1) && ((right & PAGE_WRITE) == 0)) {
>
> They are just two type of one code...
>
No, (rw & 0) always evaluates to 0.
>
> > The second
> > part removes a non-conforming behavior for the first page of the memory.
>
> @@ -122,13 +122,6 @@ static int cpu_openrisc_get_phys_addr(OpenRISCCPU
> *cpu,
> {
> int ret = TLBRET_MATCH;
>
> - /* [0x0000--0x2000]: unmapped */
> - if (address < 0x2000 && (cpu->env.sr & SR_SM)) {
> - *physical = address;
> - *prot = PAGE_READ | PAGE_WRITE;
> - return ret;
> - }
> -
>
> May you please explain more about why the first page is non-conforming?
> The Arch manual told me 0x0000--0x2000 is unmapped.
>
It shows an example where *software* leaves 0x0000 - 0x2000 unmapped,
the hardware should still allow for this area to be mapped.
Stefan
[-- Attachment #2: Type: text/html, Size: 2394 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [OpenRISC] [PATCH] Correction of the TLB handling of the OpenRISC target
2013-10-02 6:15 ` Stefan Kristiansson
@ 2013-10-03 1:42 ` Jia Liu
0 siblings, 0 replies; 5+ messages in thread
From: Jia Liu @ 2013-10-03 1:42 UTC (permalink / raw)
To: Stefan Kristiansson
Cc: Sebastian Macke, openrisc, openrisc@lists.opencores.org,
qemu-devel@nongnu.org
On Wed, Oct 2, 2013 at 2:15 PM, Stefan Kristiansson
<stefan.kristiansson@saunalahti.fi> wrote:
> On Wed, Oct 2, 2013 at 8:33 AM, Jia Liu <proljc@gmail.com> wrote:
>>
>> Hi Sebastian,
>>
>> On Wed, Oct 2, 2013 at 1:12 PM, Sebastian Macke <sebastian@macke.de>
>> wrote:
>> > Hi,
>> >
>> > this patch corrects two problems for the OpenRISC Target in QEMU. The
>> > first
>> > one corrects one obvious bug
>> > concerning the handling of page faults while reading from a page.
>>
>> @@ -102,7 +102,7 @@ int cpu_openrisc_get_phys_data(OpenRISCCPU *cpu,
>> }
>> }
>>
>> - if ((rw & 0) && ((right & PAGE_READ) == 0)) {
>> + if (!(rw & 1) && ((right & PAGE_READ) == 0)) {
>> return TLBRET_BADADDR;
>> }
>> if ((rw & 1) && ((right & PAGE_WRITE) == 0)) {
>>
>> They are just two type of one code...
>
>
> No, (rw & 0) always evaluates to 0.
>
>>
>>
>> > The second
>> > part removes a non-conforming behavior for the first page of the memory.
>>
>> @@ -122,13 +122,6 @@ static int cpu_openrisc_get_phys_addr(OpenRISCCPU
>> *cpu,
>> {
>> int ret = TLBRET_MATCH;
>>
>> - /* [0x0000--0x2000]: unmapped */
>> - if (address < 0x2000 && (cpu->env.sr & SR_SM)) {
>> - *physical = address;
>> - *prot = PAGE_READ | PAGE_WRITE;
>> - return ret;
>> - }
>> -
>>
>> May you please explain more about why the first page is non-conforming?
>> The Arch manual told me 0x0000--0x2000 is unmapped.
>
>
> It shows an example where *software* leaves 0x0000 - 0x2000 unmapped,
> the hardware should still allow for this area to be mapped.
OK, thank you. I will send a PULL Request soon.
Reviewed-by: Jia Liu <proljc@gmail.com>
>
> Stefan
Regards,
Jia
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [OpenRISC] [PATCH] Correction of the TLB handling of the OpenRISC target
2013-10-02 5:33 ` [Qemu-devel] [OpenRISC] " Jia Liu
2013-10-02 6:15 ` Stefan Kristiansson
@ 2013-10-02 6:31 ` Sebastian Macke
1 sibling, 0 replies; 5+ messages in thread
From: Sebastian Macke @ 2013-10-02 6:31 UTC (permalink / raw)
To: Jia Liu; +Cc: openrisc, openrisc, qemu-devel@nongnu.org
Hi Jia,
On 10/1/2013 10:33 PM, Jia Liu wrote:
> Hi Sebastian,
>
> On Wed, Oct 2, 2013 at 1:12 PM, Sebastian Macke <sebastian@macke.de> wrote:
>> Hi,
>>
>> this patch corrects two problems for the OpenRISC Target in QEMU. The first
>> one corrects one obvious bug
>> concerning the handling of page faults while reading from a page.
> @@ -102,7 +102,7 @@ int cpu_openrisc_get_phys_data(OpenRISCCPU *cpu,
> }
> }
>
> - if ((rw & 0) && ((right & PAGE_READ) == 0)) {
> + if (!(rw & 1) && ((right & PAGE_READ) == 0)) {
> return TLBRET_BADADDR;
> }
> if ((rw & 1) && ((right & PAGE_WRITE) == 0)) {
>
> They are just two type of one code...
No the result of (rw&0) is always zero and therefore a logic false. The
whole comparison will therefore never be executed.
>> The second
>> part removes a non-conforming behavior for the first page of the memory.
> @@ -122,13 +122,6 @@ static int cpu_openrisc_get_phys_addr(OpenRISCCPU *cpu,
> {
> int ret = TLBRET_MATCH;
>
> - /* [0x0000--0x2000]: unmapped */
> - if (address < 0x2000 && (cpu->env.sr & SR_SM)) {
> - *physical = address;
> - *prot = PAGE_READ | PAGE_WRITE;
> - return ret;
> - }
> -
>
> May you please explain more about why the first page is non-conforming?
> The Arch manual told me 0x0000--0x2000 is unmapped.
There is a good reason why this page should be a mapped one.
Think of an accidental
int *a = NULL;
....
*a = 0;
In the current implementation this would work in the supervisor mode and
could never be catched.
The Linux kernel handles this page as a mapped one without write access
which is correct.
The specification is not clear about this mapping region because it is
not consistent with the rest of the specification and of the
implementation in or1ksim and the Linux kernel.
>> I have tested this patch with the newest Linux kernel and compared the
>> output with or1ksim.
> May you please upload a newest Linux kernel to somewhere?
Try http://www.simulationcorner.net/vmlinux
start it with
or32-system-qemu -m 64 -kernel vmlinux
In / there are two example executables which accidently dereferences a
null pointer in the Linux kernel. The correct behavior should be a
kernel oops.
Sebastian
>> Sebastian
>>
>> _______________________________________________
>> OpenRISC mailing list
>> OpenRISC@lists.openrisc.net
>> http://lists.openrisc.net/listinfo/openrisc
>>
> Regards,
> Jia
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-03 1:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-02 5:12 [Qemu-devel] [PATCH] Correction of the TLB handling of the OpenRISC target Sebastian Macke
2013-10-02 5:33 ` [Qemu-devel] [OpenRISC] " Jia Liu
2013-10-02 6:15 ` Stefan Kristiansson
2013-10-03 1:42 ` Jia Liu
2013-10-02 6:31 ` Sebastian Macke
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).