Re: [x86 pmap changes] CVS commit: src/sys/arch

2020-01-08 Thread Andrew Doran
On Tue, Jan 07, 2020 at 09:39:22AM +0100, Maxime Villard wrote:

> > Module Name:src
> > Committed By:   ad
> > Date:   Sat Jan  4 22:49:20 UTC 2020
> > 
> > Modified Files:
> > src/sys/arch/x86/include: pmap.h pmap_pv.h
> > src/sys/arch/x86/x86: pmap.c
> > src/sys/arch/xen/x86: xen_pmap.c
> > 
> > Log Message:
> > x86 pmap improvements, reducing system time during a build by about 15% on
> > my test machine:
> 
> This breaks nvmm-intel. I have only given a quick glance, but this change
> already is wrong:
> 
> - old_pp->pp_attrs |= pmap_ept_to_pp_attrs(opte);
> + old_pp->pp_attrs |= pmap_pte_to_pp_attrs(opte);
> 
> This is an EPT function handling EPT PTEs, so "ept" was correct. Fixing
> this bug is not sufficient, so it seems that there are more bugs.
> 
> Reverting the whole change puts nvmm-intel back in a functional state.
> 
> You can test with this on an Intel CPU:
> 
>   # modload nvmm
>   # /usr/tests/lib/libnvmm/./h_mem_assist
> 
> This currently gives random crashes.

With a couple of typos fixed (PTE -> EPT, now checked in) I see the same FPU
DNA exception that Chavdar reports on current-users (in his case with a
kernel which doesn't have these pmap changes).  It's coming from:

vmx_vcpu_guest_fpu_leave() -> fpu_area_save() -> fxsave()

What I can tell you is that the fxsave area is definitely writable and
correctly aligned but beyond that I have no idea what's causing it.  Any
suggestions?

Cheers,
Andrew


Re: CVS commit: src/sys/arch/amd64

2020-01-08 Thread Ryo ONODERA
Hi,

However I need multiboot support for amd64.
I am waiting well-tested implementation.

"Emmanuel Dreyfus"  writes:

> Module Name:  src
> Committed By: manu
> Date: Thu Jan  9 00:42:24 UTC 2020
>
> Modified Files:
>   src/sys/arch/amd64/amd64: locore.S machdep.c
>   src/sys/arch/amd64/conf: GENERIC files.amd64 kern.ldscript
>
> Log Message:
> Rollback multiboot2 for amd64, as requested by core
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.197 -r1.198 src/sys/arch/amd64/amd64/locore.S
> cvs rdiff -u -r1.344 -r1.345 src/sys/arch/amd64/amd64/machdep.c
> cvs rdiff -u -r1.553 -r1.554 src/sys/arch/amd64/conf/GENERIC
> cvs rdiff -u -r1.114 -r1.115 src/sys/arch/amd64/conf/files.amd64
> cvs rdiff -u -r1.30 -r1.31 src/sys/arch/amd64/conf/kern.ldscript
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
> Modified files:
>
> Index: src/sys/arch/amd64/amd64/locore.S
> diff -u src/sys/arch/amd64/amd64/locore.S:1.197 
> src/sys/arch/amd64/amd64/locore.S:1.198
> --- src/sys/arch/amd64/amd64/locore.S:1.197   Wed Jan  8 20:59:18 2020
> +++ src/sys/arch/amd64/amd64/locore.S Thu Jan  9 00:42:24 2020
> @@ -1,4 +1,4 @@
> -/*   $NetBSD: locore.S,v 1.197 2020/01/08 20:59:18 skrll Exp $   */
> +/*   $NetBSD: locore.S,v 1.198 2020/01/09 00:42:24 manu Exp $*/
>  
>  /*
>   * Copyright-o-rama!
> @@ -158,7 +158,6 @@
>  
>  #include "opt_compat_netbsd.h"
>  #include "opt_compat_netbsd32.h"
> -#include "opt_multiboot.h"
>  #include "opt_xen.h"
>  #include "opt_svs.h"
>  
> @@ -178,13 +177,6 @@
>  #include 
>  #include 
>  
> -#ifndef XENPV
> -#include 
> -#endif 
> -
> -#define CODE_SEGMENT 0x08
> -#define DATA_SEGMENT 0x10
> -
>  #if NLAPIC > 0
>  #include 
>  #endif
> @@ -432,50 +424,6 @@ END(farjmp64)
>   .space  512
>  tmpstk:
>  
> -.section multiboot,"a"
> -#if defined(MULTIBOOT)
> - .align  8
> - .globl  Multiboot2_Header
> -_C_LABEL(Multiboot2_Header):
> - .intMULTIBOOT2_HEADER_MAGIC
> - .intMULTIBOOT2_ARCHITECTURE_I386
> - .intMultiboot2_Header_end - Multiboot2_Header
> - .int-(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 \
> - + (Multiboot2_Header_end - Multiboot2_Header))
> -
> - .int1   /* MULTIBOOT_HEADER_TAG_INFORMATION_REQUEST */
> - .int12  /* sizeof(multiboot_header_tag_information_request) */
> - /* + sizeof(uint32_t) * requests */
> - .int4   /* MULTIBOOT_TAG_TYPE_BASIC_MEMINFO */
> - .align  8
> -
> - .int3   /* MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS */
> - .int16  /* sizeof(struct multiboot_tag_efi64) */
> - .quad   (multiboot2_entry - KERNBASE)
> - .align  8
> -
> - .int9   /* MULTIBOOT_HEADER_TAG_ENTRY_ADDRESS_EFI64 */
> - .int16  /* sizeof(struct multiboot_tag_efi64) */
> - .quad   (multiboot2_entry - KERNBASE)
> - .align  8
> -
> -#if notyet
> - /*
> -  * Could be used to get an early console for debug,
> -  * but this is broken.
> -  */
> - .int7   /* MULTIBOOT_HEADER_TAG_EFI_BS */
> - .int8   /* sizeof(struct multiboot_tag) */
> - .align  8
> -#endif
> -
> - .int0   /* MULTIBOOT_HEADER_TAG_END */
> - .int8   /* sizeof(struct multiboot_tag) */
> - .align  8
> - .globl  Multiboot2_Header_end
> -_C_LABEL(Multiboot2_Header_end):
> -#endif   /* MULTIBOOT */
> -
>  /*
>   * Some hackage to deal with 64bit symbols in 32 bit mode.
>   * This may not be needed if things are cleaned up a little.
> @@ -492,700 +440,6 @@ ENTRY(start)
>   /* Warm boot */
>   movw$0x1234,0x472
>  
> -#if defined(MULTIBOOT)
> - jmp .Lnative_loader
> -
> -
> -multiboot2_entry:
> - .code64
> - /*
> -  * multiboot2 entry point. We are left here without
> -  * stack and with no idea of where we were loaded in memory.
> -  * The only inputs are
> -  * %eax MULTIBOOT2_BOOTLOADER_MAGIC
> -  * %ebx pointer to multiboot_info
> -  *
> -  * Here we will:
> -  * - copy the kernel to 0x20 (KERNTEXTOFF - KERNBASE)
> -  *  as almost all the code in locore.S assume it is there. 
> -  *  This is derived from 
> -  *  src/sys/arch/i386/stand/efiboot/bootx64/startprog64.S
> -  * - copy multiboot_info, as done in multiboot_pre_reloc() from
> -  *  src/sys/arch/x86/x86/multiboot2.c
> -  *  Unfortunately we cannot call that function as there is 
> -  *  no simple way to build it as 32 bit code in a 64 bit kernel.
> -  * - Copy ELF symbols, also as in multiboot_pre_reloc()
> -  */
> -
> - cli
> -
> - /*
> -  * Discover our load address and use it to get start address
> -  */
> - mov $_RELOC(tmpstk),%rsp
> - callnext
> -next:pop %r8
> - sub $(next - start), %r8
> -
> - /*
> -  * Save multiboot_info for later. We

Re: CVS commit: src/sys/arch/amd64

2020-01-08 Thread Emmanuel Dreyfus
Ryo ONODERA  wrote:

> However I need multiboot support for amd64.
> I am waiting well-tested implementation.

At this point the problems are more about code style and cleaning, as we
have a fix for the boot bugs that has been reported. 

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
m...@netbsd.org