* [Xen-devel] [PATCH] xen/arm: remove special dom0 case in dump_hyp_walk
@ 2019-10-15 3:49 Stefano Stabellini
2019-10-15 4:21 ` Jürgen Groß
2019-10-15 10:58 ` Julien Grall
0 siblings, 2 replies; 6+ messages in thread
From: Stefano Stabellini @ 2019-10-15 3:49 UTC (permalink / raw)
To: julien.grall; +Cc: jgross, xen-devel, sstabellini
There is no need to have a special dom0 case to convert the pgtable
virtual address into a physical address. The virt_to_maddr function can
work both in the dom0 case and the domU case.
This is more than a cleanup: when Xen is loaded at addresses lower than
2MB on arm32 phys_offset might not hold the right value and be unable to
perform a virt to phys conversion properly. Reducing the unnecessary
usage of phys_offset is a good idea.
Link: https://marc.info/?l=xen-devel&m=157081398022401
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index a6637ce347..b7883cfbab 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -284,10 +284,7 @@ void dump_hyp_walk(vaddr_t addr)
"on CPU%d via TTBR 0x%016"PRIx64"\n",
addr, smp_processor_id(), ttbr);
- if ( smp_processor_id() == 0 )
- BUG_ON( (lpae_t *)(unsigned long)(ttbr - phys_offset) != pgtable );
- else
- BUG_ON( virt_to_maddr(pgtable) != ttbr );
+ BUG_ON( virt_to_maddr(pgtable) != ttbr );
dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Xen-devel] [PATCH] xen/arm: remove special dom0 case in dump_hyp_walk
2019-10-15 3:49 [Xen-devel] [PATCH] xen/arm: remove special dom0 case in dump_hyp_walk Stefano Stabellini
@ 2019-10-15 4:21 ` Jürgen Groß
2019-10-15 15:50 ` Stefano Stabellini
2019-10-15 10:58 ` Julien Grall
1 sibling, 1 reply; 6+ messages in thread
From: Jürgen Groß @ 2019-10-15 4:21 UTC (permalink / raw)
To: Stefano Stabellini, julien.grall; +Cc: xen-devel
On 15.10.19 05:49, Stefano Stabellini wrote:
> There is no need to have a special dom0 case to convert the pgtable
> virtual address into a physical address. The virt_to_maddr function can
> work both in the dom0 case and the domU case.
>
> This is more than a cleanup: when Xen is loaded at addresses lower than
> 2MB on arm32 phys_offset might not hold the right value and be unable to
> perform a virt to phys conversion properly. Reducing the unnecessary
> usage of phys_offset is a good idea.
>
> Link: https://marc.info/?l=xen-devel&m=157081398022401
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index a6637ce347..b7883cfbab 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -284,10 +284,7 @@ void dump_hyp_walk(vaddr_t addr)
> "on CPU%d via TTBR 0x%016"PRIx64"\n",
> addr, smp_processor_id(), ttbr);
>
> - if ( smp_processor_id() == 0 )
> - BUG_ON( (lpae_t *)(unsigned long)(ttbr - phys_offset) != pgtable );
> - else
> - BUG_ON( virt_to_maddr(pgtable) != ttbr );
> + BUG_ON( virt_to_maddr(pgtable) != ttbr );
> dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
> }
I can't make a connection between commit message ("special dom0 case")
and the code modification. The special case removed is about cpu 0, not
dom0.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Xen-devel] [PATCH] xen/arm: remove special dom0 case in dump_hyp_walk
2019-10-15 4:21 ` Jürgen Groß
@ 2019-10-15 15:50 ` Stefano Stabellini
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Stabellini @ 2019-10-15 15:50 UTC (permalink / raw)
To: Jürgen Groß; +Cc: xen-devel, julien.grall, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]
On Tue, 15 Oct 2019, Jürgen Groß wrote:
> On 15.10.19 05:49, Stefano Stabellini wrote:
> > There is no need to have a special dom0 case to convert the pgtable
> > virtual address into a physical address. The virt_to_maddr function can
> > work both in the dom0 case and the domU case.
> >
> > This is more than a cleanup: when Xen is loaded at addresses lower than
> > 2MB on arm32 phys_offset might not hold the right value and be unable to
> > perform a virt to phys conversion properly. Reducing the unnecessary
> > usage of phys_offset is a good idea.
> >
> > Link: https://marc.info/?l=xen-devel&m=157081398022401
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> >
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index a6637ce347..b7883cfbab 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -284,10 +284,7 @@ void dump_hyp_walk(vaddr_t addr)
> > "on CPU%d via TTBR 0x%016"PRIx64"\n",
> > addr, smp_processor_id(), ttbr);
> > - if ( smp_processor_id() == 0 )
> > - BUG_ON( (lpae_t *)(unsigned long)(ttbr - phys_offset) != pgtable );
> > - else
> > - BUG_ON( virt_to_maddr(pgtable) != ttbr );
> > + BUG_ON( virt_to_maddr(pgtable) != ttbr );
> > dump_pt_walk(ttbr, addr, HYP_PT_ROOT_LEVEL, 1);
> > }
>
> I can't make a connection between commit message ("special dom0 case")
> and the code modification. The special case removed is about cpu 0, not
> dom0.
That's what happen when I write a commit message late in the night.
Sorry about that. I'll resend it.
[-- Attachment #2: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] xen/arm: remove special dom0 case in dump_hyp_walk
2019-10-15 3:49 [Xen-devel] [PATCH] xen/arm: remove special dom0 case in dump_hyp_walk Stefano Stabellini
2019-10-15 4:21 ` Jürgen Groß
@ 2019-10-15 10:58 ` Julien Grall
2019-10-15 15:51 ` Stefano Stabellini
1 sibling, 1 reply; 6+ messages in thread
From: Julien Grall @ 2019-10-15 10:58 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: jgross, xen-devel
Hi Stefano,
On 10/15/19 4:49 AM, Stefano Stabellini wrote:
> There is no need to have a special dom0 case to convert the pgtable
> virtual address into a physical address. The virt_to_maddr function can
> work both in the dom0 case and the domU case.
>
> This is more than a cleanup: when Xen is loaded at addresses lower than
> 2MB on arm32 phys_offset might not hold the right value and be unable to
> perform a virt to phys conversion properly. Reducing the unnecessary
> usage of phys_offset is a good idea.
Aside what Juergen said, this paragraph raises the question why
phys_offset is actually not modified (or completely dropped)? After all,
if the value holds is wrong then other users may get wrong value as well...
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] xen/arm: remove special dom0 case in dump_hyp_walk
2019-10-15 10:58 ` Julien Grall
@ 2019-10-15 15:51 ` Stefano Stabellini
2019-10-15 15:59 ` Julien Grall
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Stabellini @ 2019-10-15 15:51 UTC (permalink / raw)
To: Julien Grall; +Cc: jgross, xen-devel, Stefano Stabellini
On Tue, 15 Oct 2019, Julien Grall wrote:
> Hi Stefano,
>
> On 10/15/19 4:49 AM, Stefano Stabellini wrote:
> > There is no need to have a special dom0 case to convert the pgtable
> > virtual address into a physical address. The virt_to_maddr function can
> > work both in the dom0 case and the domU case.
> >
> > This is more than a cleanup: when Xen is loaded at addresses lower than
> > 2MB on arm32 phys_offset might not hold the right value and be unable to
> > perform a virt to phys conversion properly. Reducing the unnecessary
> > usage of phys_offset is a good idea.
>
> Aside what Juergen said, this paragraph raises the question why phys_offset is
> actually not modified (or completely dropped)? After all, if the value holds
> is wrong then other users may get wrong value as well...
Should I add something like "this is not a fix, but reducing unnecessary[...]" ?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Xen-devel] [PATCH] xen/arm: remove special dom0 case in dump_hyp_walk
2019-10-15 15:51 ` Stefano Stabellini
@ 2019-10-15 15:59 ` Julien Grall
0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2019-10-15 15:59 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: jgross, xen-devel
Hi Stefano,
On 10/15/19 4:51 PM, Stefano Stabellini wrote:
> On Tue, 15 Oct 2019, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 10/15/19 4:49 AM, Stefano Stabellini wrote:
>>> There is no need to have a special dom0 case to convert the pgtable
>>> virtual address into a physical address. The virt_to_maddr function can
>>> work both in the dom0 case and the domU case.
>>>
>>> This is more than a cleanup: when Xen is loaded at addresses lower than
>>> 2MB on arm32 phys_offset might not hold the right value and be unable to
>>> perform a virt to phys conversion properly. Reducing the unnecessary
>>> usage of phys_offset is a good idea.
>>
>> Aside what Juergen said, this paragraph raises the question why phys_offset is
>> actually not modified (or completely dropped)? After all, if the value holds
>> is wrong then other users may get wrong value as well...
>
> Should I add something like "this is not a fix, but reducing unnecessary[...]" ?
dump_hyp_walk() is only used in two places:
- when hardware translation failed
- Hypervisor data abort
In both case we will panic afterwards. So hitting the BUG_ON() is just
only a matter of losing page-table information.
So I see limited reason to see this patch alone in Xen 4.13. We should
either completely fix phys_offset or do nothing.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-15 16:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-15 3:49 [Xen-devel] [PATCH] xen/arm: remove special dom0 case in dump_hyp_walk Stefano Stabellini
2019-10-15 4:21 ` Jürgen Groß
2019-10-15 15:50 ` Stefano Stabellini
2019-10-15 10:58 ` Julien Grall
2019-10-15 15:51 ` Stefano Stabellini
2019-10-15 15:59 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.