Re: [Minios-devel] [PATCH 05/40] arm64: fix the wrong mask for to_virt/to_phys

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 06/11/17 09:48, Huang Shijie wrote:
> On Fri, Nov 03, 2017 at 03:03:16PM +0000, Julien Grall wrote:
> > On 03/11/17 03:11, Huang Shijie wrote:
> > > +#endif
> > > +
> > > +#define to_phys(x)                 (((paddr_t)(x)-physical_address_offset) & 
> > > ADDR_MASK)
> > Why did you switch from an addition to a subtraction?
> >
> > > +#define to_virt(x)                 ((void *)(((x)+physical_address_offset) & 
> > > ADDR_MASK))
> > Why did you switch from a subtraction to an addition?
> I do not have an arm32 platform.
Well, you said you tested on softiron which support 32-bit guests. So 
what's the problem?
But I am struggling to understand why in some places you say: "This does 
not work for arm32". Implying you tested it. And here you say: "I don't 
have any platform".
AFAICT, there are no way to compile Arm with current master:

make TARGET_ARCH_FAM=arm 


Config.mk:93: /home/julieng/works/mini-os/arch/arm/arch.mk: No such file 
or directory
make: *** No rule to make target 
'/home/julieng/works/mini-os/arch/arm/arch.mk'.  Stop.
So what did you exactly do? Do you have patch on top?

> So I can add #ifdef for the
> to_phys/to_virt, such as:
>
> #ifdef __arm__
> #define to_phys(x)                 (((paddr_t)(x)+physical_address_offset) & 
> 0xffffffff)
> #define to_virt(x)                 ((void *)(((x)-physical_address_offset) & 
> 0xffffffff))
> #ifdef __aarch64__
> #define to_phys(x)                 (((paddr_t)(x)-physical_address_offset) & 
> ADDR_MASK)
> #define to_virt(x)                 ((void *)(((x)+physical_address_offset) & 
> ADDR_MASK))
> #endif
>
> What about this solution?
That's a NACK from my side. There are strictly no reason to diverge on 
that between 32-bit and 64-bits. More that if you read my answer to the 
cover letter, I want to see some cohesion between the 2 ports.
This is totally against that idea.

Cheers,

--
Julien Grall

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