Re: [PATCH] target/i386: Fix physical address truncation when PAE is enabled

[Michael Brown]

On 20/12/2023 21:51, Richard Henderson wrote:
> On 12/20/23 22:03, Michael Brown wrote:
> > For the default case, I think it would make sense to unconditionally 
> > truncate the address to 32 bits if paging is disabled.  (I am not sure 
> > why the original commit 33dfdb5 included a test for long mode, since I 
> > do not see how it is possible to get the CPU into long mode with 
> > paging disabled.)
>
> You are correct that paging is mandatory for LMA -- indeed, setting 
> CR0.PG is the final step in 10.8.5 Initializing IA-32e Mode, which 
> copies EFER.LME to EFER.LMA.
>
> The commit 33dfdb5 that you reference is definitely wrong.

I have done some further investigation, and come to the conclusion that 
we should just delete the truncation code entirely.

With paging disabled, there is (as far as I know) no way to execute an 
instruction with an address size of 64 bits.  With the instruction 
address size being 32 bits, the linear address will already be truncated 
to 32 bits anyway.

A quick userspace test program confirms that on a physical CPU a 32-bit 
address size will result in the linear address being truncated to 32 bits:

    #include <stdint.h>
    #include <stdio.h>

    uint8_t val = 42;

    int main ( void ) {
        uint8_t *ptr = ( &val + 0x80000001 );
        uint8_t res;

        printf ( "&val = %p\n", &val );  // will be around 0x400000
        printf ( "ptr = %p\n", ptr );

        printf ( "addr32 read via 0x7fffffff(%p)...\n", ptr );
        __asm__ ( "addr32 movb 0x7fffffff(%k1), %b0\n\t"
                  : "=r" ( res ) : "r" ( ptr ) );
        printf ( "...got %d\n", res );

        printf ( "addr64 read via 0x7fffffff(%p)...\n", ptr );
        __asm__ ( "movb 0x7fffffff(%1), %b0\n\t"
                  : "=r" ( res ) : "r" ( ptr ) );
        printf ( "...got %d\n", res );

        return 0;
    }

produces the expected output:

$ cc -o test test.c && ./test
&val = 0x40400c
ptr = 0x8040400d
addr32 read via 0x7fffffff(0x8040400d)...
...got 42
addr64 read via 0x7fffffff(0x8040400d)...
Segmentation fault (core dumped)

which I believe shows that the addr32 instruction experiences wraparound 
of the linear address (and so accesses the correct location), while the 
addr64 instruction with the same base and offset does not experience 
wraparound of the linear address (and so segfaults).

The original commit 33dfdb5 describes 32-bit code relying on the linear 
address wraparound to access memory below the segment base.  This is 
what iPXE also does: the iPXE code is physically copied to somewhere 
high in 32-bit memory, the segment base is set to point to the location 
of iPXE (thereby avoiding the need to apply relocation records), and 
wraparound is relied upon to access all memory below this.  I have 
tested removing the truncation code from get_physical_address() and 
verified that iPXE continues to work as expected.

I have also verified that removing the truncation code from 
get_physical_address() does seem to fix the problem with PAE mode (i.e. 
is able to boot Windows 10 with RAM above 4G).

I will send a v2 patch shortly.

Thanks,

Michael