Re: [Minios-devel] [PATCH v3 08/43] arm64: fix the wrong mask for to_virt/to_phys

[Julien Grall <julien.grall@xxxxxxx>]

On 19/04/18 11:20, Huang Shijie wrote:
> On Thu, Apr 19, 2018 at 11:09:30AM +0100, Julien Grall wrote:
> > On 19/04/18 11:01, Huang Shijie wrote:
> > > On Wed, Apr 18, 2018 at 05:42:37PM +0100, Julien Grall wrote:
> > > > Hi,
> > > >
> > > > On 16/04/18 07:31, Huang Shijie wrote:
> > > > > In the arm64, the mask 0xffffffff will truncate the value, and
> > > > s/In the/On/
> > > >
> > > > > to_virt/to_phys will get wrong results.
> > > > >
> > > > > This patch fixes it.
> > > > >
> > > > > Signed-off-by: Huang Shijie <shijie.huang@xxxxxxx>
> > > > > ---
> > > > >   include/arm/arch_mm.h | 4 ++--
> > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/include/arm/arch_mm.h b/include/arm/arch_mm.h
> > > > > index c66856c..f77a210 100644
> > > > > --- a/include/arm/arch_mm.h
> > > > > +++ b/include/arm/arch_mm.h
> > > > > @@ -15,8 +15,8 @@ extern paddr_t physical_address_offset;
> > > > >   #define DEF_PAGE_PROT     0
> > > > > -#define to_phys(x)                 (((paddr_t)(x)+physical_address_offset) & 
> > > > > 0xffffffff)
> > > > > -#define to_virt(x)                 ((void *)(((x)-physical_address_offset) & 
> > > > > 0xffffffff))
> > > > > +#define to_phys(x)                 (((paddr_t)(x)+physical_address_offset) & 
> > > > > (~0UL))
> > > > While this looks good on arm64, for arm32 unsigned long is not going to be
> > > > enough. So I think you want to use ~(paddr_t)0 to cover the 32-bit case.
> > > Sorry, I do not understand your meaning... :(
> > > Do we need to cover the arm32 here?
> > to_phys() and to_virt() are defined in generic arm headers. While 32-bit is
> > not supported, it would be better to cover it as cast problem are really a
> > pain to fix it afterwards.
> Okay, I will use "~(paddr_t)0" for the to_phys...
>
> > But I just realized that mini-os only support 4G of RAM because it relies on
> > mapping all the memory. Am I right?
> I did not try big memory which is over 4G. But I think the mini-os can support 
> memory
> over 4G.
I meant on 32-bit. Sorry for not been clearer.

> > If so, the UL might be fine. Any opinions?
> Dido, I will use "~(paddr_t)0" for the to_phys...
>
> Thanks
> Huang Shijie
--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel