Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr
On 01/25/19 at 03:08pm, Borislav Petkov wrote: > On Fri, Jan 25, 2019 at 09:45:18PM +0800, Dave Young wrote: > > AFAIK, some people prefer to explictly reserve crash memory at high > > region even if it is possible to reserve at low area. May because > > <4G memory is limited on large server, they want to leave this for other > > use. > > > > Yinghai or Vivek should know more about the history, probably they can > > recall some initial reason. > > Yes, just "prefer" is not good enough. There should be a technical > reason why that's there. > > Also, if the user doesn't care, then the code should be free to force > "high" and thus probe a different range for allocation. Another reason is in case ,high we will need automatically reserve a region in low area for swiotlb. So for example one use crashkernel=256M,high, actual reserved memory is 256M above 4G and another 256M under 4G for swiotlb. Normally it is not necessary for most people. Thus we can not make ,high as default. Thanks Dave ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr
On Mon, Jan 28, 2019 at 05:58:09PM +0800, Dave Young wrote: > Another reason is in case ,high we will need automatically reserve a > region in low area for swiotlb. So for example one use > crashkernel=256M,high, actual reserved memory is 256M above 4G and > another 256M under 4G for swiotlb. Normally it is not necessary for > most people. Thus we can not make ,high as default. And how is the poor user to figure out that we decided for her/him that swiotlb reservation is something not necessary for most people and thus we fail the crashkernel= reservation? IOW, that "logic" above doesn't make a whole lot of sense to me from user friendliness perspective. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: multiboot patches from august 2018
On Mon, Jan 21, 2019 at 08:07:32PM +0100, cinap_len...@felloff.net wrote: > i'm unsubscribing from this list as i did not get any > response about these patches from anyone but horms so far > and that was a long time ago. > > http://lists.infradead.org/pipermail/kexec/2018-August/021357.html > http://lists.infradead.org/pipermail/kexec/2018-August/021358.html > http://lists.infradead.org/pipermail/kexec/2018-August/021359.html > > if there is any questions please email me directly. Thanks, and sorry for the long delay. I have applied these patches for inclusion in the next release of kexec-tools. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH] purgatory: Use standalond CFLAGS
There has been a lot of workarounds for purgatory disabling many specified CFLAGS that will break purgatory. It will be better to not let the CFLAGS used to compile purgatory honor the CFLAGS from environment variables. So we will have stable CFLAGS for purgatory. If anyone still wants to change purgatory CFLAGS, PURGATORY_EXTRA_CFLAGS is still honored. --- purgatory/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/purgatory/Makefile b/purgatory/Makefile index 49ce80a..2dd6c47 100644 --- a/purgatory/Makefile +++ b/purgatory/Makefile @@ -45,7 +45,7 @@ purgatory/sha256.o: $(srcdir)/util_lib/sha256.c $(COMPILE.c) -o $@ $^ $(PURGATORY): CC=$(TARGET_CC) -$(PURGATORY): CFLAGS+=$(PURGATORY_EXTRA_CFLAGS) \ +$(PURGATORY): CFLAGS=$(PURGATORY_EXTRA_CFLAGS) \ $($(ARCH)_PURGATORY_EXTRA_CFLAGS) \ -Os -fno-builtin -ffreestanding \ -fno-zero-initialized-in-bss \ -- 2.20.1 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH] makedumpfile: honor the CFLAGS from environment variables
This make it easier for passing extra cflags, for example hardening flags could be passed in with enviroment variable. Signed-off-by: Kairui Song --- Makefile | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 612b9d0..b511a78 100644 --- a/Makefile +++ b/Makefile @@ -8,11 +8,11 @@ ifeq ($(strip $CC),) CC = gcc endif -CFLAGS = -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ - -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' -CFLAGS_ARCH= -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE +CFLAGS_ARCH = $(CFLAGS) -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ + -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE +CFLAGS += -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ +-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ +-DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' # LDFLAGS = -L/usr/local/lib -I/usr/local/include HOST_ARCH := $(shell uname -m) -- 2.20.1 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86: Handle 64bit framebuffer memory address properly
On Tue, Jan 22, 2019 at 10:25:42AM +0800, Kairui Song wrote:
> On Wed, Dec 5, 2018 at 5:22 PM Kairui Song wrote:
> >
> > In a EFI system, the frame buffer address is 64bit, so currently
> > if the address is beyound 4G, kexec will set wrong address due to
> > truncate.
> >
> > Linux kernel commit ae2ee627dc87 ('efifb: Add support for 64-bit
> > frame buffer addresses') added support for 64bit frame buffer
> > address, an 'ext_lfb_base' field is added as the upper 32-bits of
> > the frame buffer, and introduced a new capability flag
> > 'VIDEO_TYPE_CAPABILITY_64BIT_BASE' to indicate if the extend field is
> > used.
> >
> > This patch adopts this change, set proper extent address and capability
> > flag when the address is beyound 4G.
> >
> > Signed-off-by: Kairui Song
> > ---
> > Update from V1:
> > - Use 0xUL instead of 0x for comparing and bit operation
> > with smem_start to make it cleaner and safer, as smem_start is an
> > unsigned long.
> >
> > include/x86/x86-linux.h | 7 +--
> > kexec/arch/i386/x86-linux-setup.c | 7 ++-
> > 2 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
> > index 7834751..352ea02 100644
> > --- a/include/x86/x86-linux.h
> > +++ b/include/x86/x86-linux.h
> > @@ -107,7 +107,10 @@ struct x86_linux_param_header {
> > uint16_t vesapm_seg;/* 0x2e */
> > uint16_t vesapm_off;/* 0x30 */
> > uint16_t pages; /* 0x32 */
> > - uint8_t reserved4[12]; /* 0x34 -- 0x3f reserved
> > for future expansion */
> > + uint16_t vesa_attributes; /* 0x34 */
> > + uint32_t capabilities; /* 0x36 */
> > + uint32_t ext_lfb_base; /* 0x3a */
> > + uint8_t reserved4[2]; /* 0x3e -- 0x3f reserved
> > for future expansion */
> >
> > struct apm_bios_info apm_bios_info; /* 0x40 */
> > struct drive_info_struct drive_info;/* 0x80 */
> > @@ -199,7 +202,7 @@ struct x86_linux_param_header {
> > struct edd_info eddbuf[EDDMAXNR]; /* 0xd00 */
> > /* 0xeec */
> > #define COMMAND_LINE_SIZE 2048
> > -};
> > +} __attribute__((packed));
> >
> > struct x86_linux_faked_param_header {
> > struct x86_linux_param_header hdr; /* 0x00 */
> > diff --git a/kexec/arch/i386/x86-linux-setup.c
> > b/kexec/arch/i386/x86-linux-setup.c
> > index 6cda12c..1bd408b 100644
> > --- a/kexec/arch/i386/x86-linux-setup.c
> > +++ b/kexec/arch/i386/x86-linux-setup.c
> > @@ -158,10 +158,15 @@ static int setup_linux_vesafb(struct
> > x86_linux_param_header *real_mode)
> > real_mode->lfb_width = var.xres;
> > real_mode->lfb_height = var.yres;
> > real_mode->lfb_depth = var.bits_per_pixel;
> > - real_mode->lfb_base = fix.smem_start;
> > + real_mode->lfb_base = fix.smem_start & 0xUL;
> > real_mode->lfb_linelength = fix.line_length;
> > real_mode->vesapm_seg = 0;
> >
> > + if (fix.smem_start > 0xUL) {
> > + real_mode->ext_lfb_base = fix.smem_start >> 32;
> > + real_mode->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
> > + }
> > +
> > /* FIXME: better get size from the file returned by proc_iomem() */
> > real_mode->lfb_size = (fix.smem_len + 65535) / 65536;
> > real_mode->pages = (fix.smem_len + 4095) / 4096;
> > --
> > 2.19.1
> >
>
> Hi Simon,
>
> Could you please have a look of this patch? Thanks
Sorry, for the delay.
This got stuck in my TODO list for way to long.
This patch looks good to me and I have applied it for
inclusion in the next release of kexec-tools.
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] purgatory: Use standalond CFLAGS
On Mon, Jan 28, 2019 at 06:50:40PM +0800, Kairui Song wrote: > There has been a lot of workarounds for purgatory disabling many > specified CFLAGS that will break purgatory. It will be better to not > let the CFLAGS used to compile purgatory honor the CFLAGS from > environment variables. So we will have stable CFLAGS for purgatory. > > If anyone still wants to change purgatory CFLAGS, PURGATORY_EXTRA_CFLAGS > is still honored. Thanks, this seems reasonable to me. But I'd like to wait to see if there is feedback from others before applying. > > --- > purgatory/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/purgatory/Makefile b/purgatory/Makefile > index 49ce80a..2dd6c47 100644 > --- a/purgatory/Makefile > +++ b/purgatory/Makefile > @@ -45,7 +45,7 @@ purgatory/sha256.o: $(srcdir)/util_lib/sha256.c > $(COMPILE.c) -o $@ $^ > > $(PURGATORY): CC=$(TARGET_CC) > -$(PURGATORY): CFLAGS+=$(PURGATORY_EXTRA_CFLAGS) \ > +$(PURGATORY): CFLAGS=$(PURGATORY_EXTRA_CFLAGS) \ > $($(ARCH)_PURGATORY_EXTRA_CFLAGS) \ > -Os -fno-builtin -ffreestanding \ > -fno-zero-initialized-in-bss \ > -- > 2.20.1 > ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] makedumpfile: honor the CFLAGS from environment variables
On Mon, Jan 28, 2019 at 06:50:45PM +0800, Kairui Song wrote: > This make it easier for passing extra cflags, for example hardening > flags could be passed in with enviroment variable. > > Signed-off-by: Kairui Song Thanks, I like this a lot. I would like to wake a little to see if there is review from others before applying this. > --- > Makefile | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/Makefile b/Makefile > index 612b9d0..b511a78 100644 > --- a/Makefile > +++ b/Makefile > @@ -8,11 +8,11 @@ ifeq ($(strip $CC),) > CC = gcc > endif > > -CFLAGS = -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ > - -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' > -CFLAGS_ARCH = -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > +CFLAGS_ARCH = $(CFLAGS) -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > + -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > +CFLAGS += -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > + -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ > + -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' > # LDFLAGS = -L/usr/local/lib -I/usr/local/include > > HOST_ARCH := $(shell uname -m) > -- > 2.20.1 > > > ___ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec > ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] x86: Handle 64bit framebuffer memory address properly
On Mon, Jan 28, 2019 at 6:53 PM Simon Horman wrote:
>
> On Tue, Jan 22, 2019 at 10:25:42AM +0800, Kairui Song wrote:
> > On Wed, Dec 5, 2018 at 5:22 PM Kairui Song wrote:
> > >
> > > In a EFI system, the frame buffer address is 64bit, so currently
> > > if the address is beyound 4G, kexec will set wrong address due to
> > > truncate.
> > >
> > > Linux kernel commit ae2ee627dc87 ('efifb: Add support for 64-bit
> > > frame buffer addresses') added support for 64bit frame buffer
> > > address, an 'ext_lfb_base' field is added as the upper 32-bits of
> > > the frame buffer, and introduced a new capability flag
> > > 'VIDEO_TYPE_CAPABILITY_64BIT_BASE' to indicate if the extend field is
> > > used.
> > >
> > > This patch adopts this change, set proper extent address and capability
> > > flag when the address is beyound 4G.
> > >
> > > Signed-off-by: Kairui Song
> > > ---
> > > Update from V1:
> > > - Use 0xUL instead of 0x for comparing and bit operation
> > > with smem_start to make it cleaner and safer, as smem_start is an
> > > unsigned long.
> > >
> > > include/x86/x86-linux.h | 7 +--
> > > kexec/arch/i386/x86-linux-setup.c | 7 ++-
> > > 2 files changed, 11 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/x86/x86-linux.h b/include/x86/x86-linux.h
> > > index 7834751..352ea02 100644
> > > --- a/include/x86/x86-linux.h
> > > +++ b/include/x86/x86-linux.h
> > > @@ -107,7 +107,10 @@ struct x86_linux_param_header {
> > > uint16_t vesapm_seg;/* 0x2e */
> > > uint16_t vesapm_off;/* 0x30 */
> > > uint16_t pages; /* 0x32 */
> > > - uint8_t reserved4[12]; /* 0x34 -- 0x3f reserved
> > > for future expansion */
> > > + uint16_t vesa_attributes; /* 0x34 */
> > > + uint32_t capabilities; /* 0x36 */
> > > + uint32_t ext_lfb_base; /* 0x3a */
> > > + uint8_t reserved4[2]; /* 0x3e -- 0x3f reserved
> > > for future expansion */
> > >
> > > struct apm_bios_info apm_bios_info; /* 0x40 */
> > > struct drive_info_struct drive_info;/* 0x80 */
> > > @@ -199,7 +202,7 @@ struct x86_linux_param_header {
> > > struct edd_info eddbuf[EDDMAXNR]; /* 0xd00 */
> > > /* 0xeec */
> > > #define COMMAND_LINE_SIZE 2048
> > > -};
> > > +} __attribute__((packed));
> > >
> > > struct x86_linux_faked_param_header {
> > > struct x86_linux_param_header hdr; /* 0x00 */
> > > diff --git a/kexec/arch/i386/x86-linux-setup.c
> > > b/kexec/arch/i386/x86-linux-setup.c
> > > index 6cda12c..1bd408b 100644
> > > --- a/kexec/arch/i386/x86-linux-setup.c
> > > +++ b/kexec/arch/i386/x86-linux-setup.c
> > > @@ -158,10 +158,15 @@ static int setup_linux_vesafb(struct
> > > x86_linux_param_header *real_mode)
> > > real_mode->lfb_width = var.xres;
> > > real_mode->lfb_height = var.yres;
> > > real_mode->lfb_depth = var.bits_per_pixel;
> > > - real_mode->lfb_base = fix.smem_start;
> > > + real_mode->lfb_base = fix.smem_start & 0xUL;
> > > real_mode->lfb_linelength = fix.line_length;
> > > real_mode->vesapm_seg = 0;
> > >
> > > + if (fix.smem_start > 0xUL) {
> > > + real_mode->ext_lfb_base = fix.smem_start >> 32;
> > > + real_mode->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
> > > + }
> > > +
> > > /* FIXME: better get size from the file returned by proc_iomem()
> > > */
> > > real_mode->lfb_size = (fix.smem_len + 65535) / 65536;
> > > real_mode->pages = (fix.smem_len + 4095) / 4096;
> > > --
> > > 2.19.1
> > >
> >
> > Hi Simon,
> >
> > Could you please have a look of this patch? Thanks
>
> Sorry, for the delay.
> This got stuck in my TODO list for way to long.
>
> This patch looks good to me and I have applied it for
> inclusion in the next release of kexec-tools.
No worry, thanks for the merge.
--
Best Regards,
Kairui Song
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] Remove the memory encryption mask to obtain the true physical address
On 1/27/19 11:46 PM, Lianbo Jiang wrote:
> For AMD machine with SME feature, if SME is enabled in the first
> kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
> the memory encryption mask, so makedumpfile needs to remove the
> memory encryption mask to obtain the true physical address.
>
> Signed-off-by: Lianbo Jiang
> ---
> Changes since v1:
> 1. Merge them into a patch.
> 2. The sme_mask is not an enum number, remove it.
> 3. Sanity check whether the sme_mask is in vmcoreinfo.
> 4. Deal with the huge pages case.
> 5. Cover the 5-level path.
>
> arch/x86_64.c | 30 +-
> makedumpfile.c | 4
> makedumpfile.h | 1 +
> 3 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86_64.c b/arch/x86_64.c
> index 537fb78..7b3ed10 100644
> --- a/arch/x86_64.c
> +++ b/arch/x86_64.c
> @@ -291,6 +291,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> unsigned long page_dir, pgd, pud_paddr, pud_pte, pmd_paddr, pmd_pte;
> unsigned long pte_paddr, pte;
> unsigned long p4d_paddr, p4d_pte;
> + unsigned long sme_me_mask = ~0UL;
>
> /*
>* Get PGD.
> @@ -302,6 +303,9 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> return NOT_PADDR;
> }
>
> + if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
> + sme_me_mask = ~(NUMBER(sme_mask));
This is a bit confusing since this isn't the sme_me_mask any more, but the
complement. Might want to somehow rename this so that it doesn't cause any
confusion.
> +
> if (check_5level_paging()) {
> page_dir += pgd5_index(vaddr) * sizeof(unsigned long);
> if (!readmem(PADDR, page_dir, &pgd, sizeof pgd)) {
> @@ -309,7 +313,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> return NOT_PADDR;
> }
> if (info->vaddr_for_vtop == vaddr)
> - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
> + MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
> sme_me_mask));
No need to remove the mask here. You're just printing out the value of
the entry. It might be nice to know whether the encryption bit is set or
not - after all, ENTRY_MASK is still part of this value.
>
> if (!(pgd & _PAGE_PRESENT)) {
> ERRMSG("Can't get a valid pgd.\n");
> @@ -318,20 +322,20 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> /*
>* Get P4D.
>*/
> - p4d_paddr = pgd & ENTRY_MASK;
> + p4d_paddr = pgd & ENTRY_MASK & sme_me_mask;
This goes back to my original comment that you should just make a local
variable that is "ENTRY_MASK & ~(NUMBER(sme_mask))" since you are
performing this ANDing everywhere ENTRY_MASK is used - except then you
miss the one at the very end of this routine on the return statement.
> p4d_paddr += p4d_index(vaddr) * sizeof(unsigned long);
> if (!readmem(PADDR, p4d_paddr, &p4d_pte, sizeof p4d_pte)) {
> ERRMSG("Can't get p4d_pte (p4d_paddr:%lx).\n",
> p4d_paddr);
> return NOT_PADDR;
> }
> if (info->vaddr_for_vtop == vaddr)
> - MSG(" P4D : %16lx => %16lx\n", p4d_paddr, p4d_pte);
> + MSG(" P4D : %16lx => %16lx\n", p4d_paddr, (p4d_pte &
> sme_me_mask));
>
> if (!(p4d_pte & _PAGE_PRESENT)) {
> ERRMSG("Can't get a valid p4d_pte.\n");
> return NOT_PADDR;
> }
> - pud_paddr = p4d_pte & ENTRY_MASK;
> + pud_paddr = p4d_pte & ENTRY_MASK & sme_me_mask;
> }else {
> page_dir += pgd_index(vaddr) * sizeof(unsigned long);
> if (!readmem(PADDR, page_dir, &pgd, sizeof pgd)) {
> @@ -339,13 +343,13 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> return NOT_PADDR;
> }
> if (info->vaddr_for_vtop == vaddr)
> - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
> + MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
> sme_me_mask));
>
> if (!(pgd & _PAGE_PRESENT)) {
> ERRMSG("Can't get a valid pgd.\n");
> return NOT_PADDR;
> }
> - pud_paddr = pgd & ENTRY_MASK;
> + pud_paddr = pgd & ENTRY_MASK & sme_me_mask;
> }
>
> /*
> @@ -357,47 +361,47 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> return NOT_PADDR;
> }
> if (info->vaddr_for_vtop == vaddr)
> - MSG(" PUD : %16lx => %16lx\n", pud_paddr, pud_pte);
> + MSG(" PUD : %16lx => %16lx\n", pud_paddr, (pud_pte &
> sme_me_mask));
>
> if (!(pud_pte & _PAGE_PRESENT)) {
>
RE: [PATCH v2] Remove the memory encryption mask to obtain the true physical address
On 1/28/2019 9:24 AM, Lendacky, Thomas wrote:
> On 1/27/19 11:46 PM, Lianbo Jiang wrote:
> > For AMD machine with SME feature, if SME is enabled in the first
> > kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
> > the memory encryption mask, so makedumpfile needs to remove the
> > memory encryption mask to obtain the true physical address.
> >
> > Signed-off-by: Lianbo Jiang
> > ---
> > Changes since v1:
> > 1. Merge them into a patch.
> > 2. The sme_mask is not an enum number, remove it.
> > 3. Sanity check whether the sme_mask is in vmcoreinfo.
> > 4. Deal with the huge pages case.
> > 5. Cover the 5-level path.
> >
> > arch/x86_64.c | 30 +-
> > makedumpfile.c | 4
> > makedumpfile.h | 1 +
> > 3 files changed, 22 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/x86_64.c b/arch/x86_64.c
> > index 537fb78..7b3ed10 100644
> > --- a/arch/x86_64.c
> > +++ b/arch/x86_64.c
> > @@ -291,6 +291,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > unsigned long page_dir, pgd, pud_paddr, pud_pte, pmd_paddr, pmd_pte;
> > unsigned long pte_paddr, pte;
> > unsigned long p4d_paddr, p4d_pte;
> > + unsigned long sme_me_mask = ~0UL;
> >
> > /*
> > * Get PGD.
> > @@ -302,6 +303,9 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > return NOT_PADDR;
> > }
> >
> > + if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
> > + sme_me_mask = ~(NUMBER(sme_mask));
>
> This is a bit confusing since this isn't the sme_me_mask any more, but the
> complement. Might want to somehow rename this so that it doesn't cause any
> confusion.
>
> > +
> > if (check_5level_paging()) {
> > page_dir += pgd5_index(vaddr) * sizeof(unsigned long);
> > if (!readmem(PADDR, page_dir, &pgd, sizeof pgd)) {
> > @@ -309,7 +313,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > return NOT_PADDR;
> > }
> > if (info->vaddr_for_vtop == vaddr)
> > - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
> > + MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
> > sme_me_mask));
>
> No need to remove the mask here. You're just printing out the value of
> the entry. It might be nice to know whether the encryption bit is set or
> not - after all, ENTRY_MASK is still part of this value.
Agreed.
>
> >
> > if (!(pgd & _PAGE_PRESENT)) {
> > ERRMSG("Can't get a valid pgd.\n");
> > @@ -318,20 +322,20 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > /*
> > * Get P4D.
> > */
> > - p4d_paddr = pgd & ENTRY_MASK;
> > + p4d_paddr = pgd & ENTRY_MASK & sme_me_mask;
>
> This goes back to my original comment that you should just make a local
> variable that is "ENTRY_MASK & ~(NUMBER(sme_mask))" since you are
> performing this ANDing everywhere ENTRY_MASK is used - except then you
> miss the one at the very end of this routine on the return statement.
This was my idea I said to Lianbo before seeing your comment, but
yes, including ENTRY_MASK in a local variable is better than that.
Thanks for your good suggestion.
As for the variable's name, I think that "entry_mask" is good enough,
but any better name?
unsigned long entry_mask = ENTRY_MASK;
if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
entry_mask &= ~(NUMBER(sme_mask));
...
p4d_paddr = pgd & entry_mask;
And, I found that the find_vmemmap_x86_64() function also uses the
page table for the -e option and looks to be affected by SME.
Lianbo, would you fix the function, too?
Thanks,
Kazu
>
> > p4d_paddr += p4d_index(vaddr) * sizeof(unsigned long);
> > if (!readmem(PADDR, p4d_paddr, &p4d_pte, sizeof p4d_pte)) {
> > ERRMSG("Can't get p4d_pte (p4d_paddr:%lx).\n",
> > p4d_paddr);
> > return NOT_PADDR;
> > }
> > if (info->vaddr_for_vtop == vaddr)
> > - MSG(" P4D : %16lx => %16lx\n", p4d_paddr, p4d_pte);
> > + MSG(" P4D : %16lx => %16lx\n", p4d_paddr, (p4d_pte &
> > sme_me_mask));
> >
> > if (!(p4d_pte & _PAGE_PRESENT)) {
> > ERRMSG("Can't get a valid p4d_pte.\n");
> > return NOT_PADDR;
> > }
> > - pud_paddr = p4d_pte & ENTRY_MASK;
> > + pud_paddr = p4d_pte & ENTRY_MASK & sme_me_mask;
> > }else {
> > page_dir += pgd_index(vaddr) * sizeof(unsigned long);
> > if (!readmem(PADDR, page_dir, &pgd, sizeof pgd)) {
> > @@ -339,13 +343,13 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > return NOT_PADDR;
> > }
> > if (info->vaddr_for_vtop == vaddr)
> > - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
> > +
Re: [PATCH v2] Remove the memory encryption mask to obtain the true physical address
On 1/28/19 1:45 PM, Kazuhito Hagio wrote:
> On 1/28/2019 9:24 AM, Lendacky, Thomas wrote:
>> On 1/27/19 11:46 PM, Lianbo Jiang wrote:
>>> For AMD machine with SME feature, if SME is enabled in the first
>>> kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
>>> the memory encryption mask, so makedumpfile needs to remove the
>>> memory encryption mask to obtain the true physical address.
>>>
>>> Signed-off-by: Lianbo Jiang
>>> ---
>>> Changes since v1:
>>> 1. Merge them into a patch.
>>> 2. The sme_mask is not an enum number, remove it.
>>> 3. Sanity check whether the sme_mask is in vmcoreinfo.
>>> 4. Deal with the huge pages case.
>>> 5. Cover the 5-level path.
>>>
>>> arch/x86_64.c | 30 +-
>>> makedumpfile.c | 4
>>> makedumpfile.h | 1 +
>>> 3 files changed, 22 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/x86_64.c b/arch/x86_64.c
>>> index 537fb78..7b3ed10 100644
>>> --- a/arch/x86_64.c
>>> +++ b/arch/x86_64.c
>>> @@ -291,6 +291,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> unsigned long page_dir, pgd, pud_paddr, pud_pte, pmd_paddr, pmd_pte;
>>> unsigned long pte_paddr, pte;
>>> unsigned long p4d_paddr, p4d_pte;
>>> + unsigned long sme_me_mask = ~0UL;
>>>
>>> /*
>>> * Get PGD.
>>> @@ -302,6 +303,9 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> return NOT_PADDR;
>>> }
>>>
>>> + if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>>> + sme_me_mask = ~(NUMBER(sme_mask));
>>
>> This is a bit confusing since this isn't the sme_me_mask any more, but the
>> complement. Might want to somehow rename this so that it doesn't cause any
>> confusion.
>>
>>> +
>>> if (check_5level_paging()) {
>>> page_dir += pgd5_index(vaddr) * sizeof(unsigned long);
>>> if (!readmem(PADDR, page_dir, &pgd, sizeof pgd)) {
>>> @@ -309,7 +313,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> return NOT_PADDR;
>>> }
>>> if (info->vaddr_for_vtop == vaddr)
>>> - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
>>> + MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
>>> sme_me_mask));
>>
>> No need to remove the mask here. You're just printing out the value of
>> the entry. It might be nice to know whether the encryption bit is set or
>> not - after all, ENTRY_MASK is still part of this value.
>
> Agreed.
>
>>
>>>
>>> if (!(pgd & _PAGE_PRESENT)) {
>>> ERRMSG("Can't get a valid pgd.\n");
>>> @@ -318,20 +322,20 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> /*
>>> * Get P4D.
>>> */
>>> - p4d_paddr = pgd & ENTRY_MASK;
>>> + p4d_paddr = pgd & ENTRY_MASK & sme_me_mask;
>>
>> This goes back to my original comment that you should just make a local
>> variable that is "ENTRY_MASK & ~(NUMBER(sme_mask))" since you are
>> performing this ANDing everywhere ENTRY_MASK is used - except then you
>> miss the one at the very end of this routine on the return statement.
>
> This was my idea I said to Lianbo before seeing your comment, but
> yes, including ENTRY_MASK in a local variable is better than that.
> Thanks for your good suggestion.
>
> As for the variable's name, I think that "entry_mask" is good enough,
> but any better name?
No, I think "entry_mask" is just fine.
>
> unsigned long entry_mask = ENTRY_MASK;
>
> if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
> entry_mask &= ~(NUMBER(sme_mask));
> ...
> p4d_paddr = pgd & entry_mask;
Yup, exactly what I was thinking.
Thanks,
Tom
>
> And, I found that the find_vmemmap_x86_64() function also uses the
> page table for the -e option and looks to be affected by SME.
> Lianbo, would you fix the function, too?
>
> Thanks,
> Kazu
>
>>
>>> p4d_paddr += p4d_index(vaddr) * sizeof(unsigned long);
>>> if (!readmem(PADDR, p4d_paddr, &p4d_pte, sizeof p4d_pte)) {
>>> ERRMSG("Can't get p4d_pte (p4d_paddr:%lx).\n",
>>> p4d_paddr);
>>> return NOT_PADDR;
>>> }
>>> if (info->vaddr_for_vtop == vaddr)
>>> - MSG(" P4D : %16lx => %16lx\n", p4d_paddr, p4d_pte);
>>> + MSG(" P4D : %16lx => %16lx\n", p4d_paddr, (p4d_pte &
>>> sme_me_mask));
>>>
>>> if (!(p4d_pte & _PAGE_PRESENT)) {
>>> ERRMSG("Can't get a valid p4d_pte.\n");
>>> return NOT_PADDR;
>>> }
>>> - pud_paddr = p4d_pte & ENTRY_MASK;
>>> + pud_paddr = p4d_pte & ENTRY_MASK & sme_me_mask;
>>> }else {
>>> page_dir += pgd_index(vaddr) * sizeof(unsigned long);
>>> if (!readmem(PADDR, page_dir, &pgd, sizeof pgd)) {
>>> @@ -339,13 +343,13 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>
[PATCH 1/2] arm64: Expose address bits (physical/virtual) via cpuinfo
With ARMv8.2-LVA and LPA architecture extensions, arm64 hardware which
supports these extensions can support upto 52-bit virtual and 52-bit
physical addresses respectively.
Since at the moment we enable the support of these extensions via CONFIG
flags, e.g.
- LPA via CONFIG_ARM64_PA_BITS_52, and
- LVA via CONFIG_ARM64_FORCE_52BIT
The easiest way a user can determine the physical/virtual
addresses supported on the hardware, is via the '/proc/cpuinfo'
interface.
This patches enables the same.
Signed-off-by: Bhupesh Sharma
---
arch/arm64/include/asm/cpufeature.h | 59 -
arch/arm64/include/asm/sysreg.h | 19
arch/arm64/kernel/cpuinfo.c | 4 ++-
3 files changed, 61 insertions(+), 21 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h
b/arch/arm64/include/asm/cpufeature.h
index dfcfba725d72..2f1270ddc277 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -522,6 +522,45 @@ static inline bool system_supports_32bit_el0(void)
return cpus_have_const_cap(ARM64_HAS_32BIT_EL0);
}
+static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange)
+{
+ switch (parange) {
+ case ID_AA64MMFR0_PARANGE_32: return PARANGE_32;
+ case ID_AA64MMFR0_PARANGE_36: return PARANGE_36;
+ case ID_AA64MMFR0_PARANGE_40: return PARANGE_40;
+ case ID_AA64MMFR0_PARANGE_44: return PARANGE_44;
+ case ID_AA64MMFR0_PARANGE_48: return PARANGE_48;
+ case ID_AA64MMFR0_PARANGE_52: return PARANGE_52;
+ /*
+* A future PE could use a value unknown to the kernel.
+* However, by the "D10.1.4 Principles of the ID scheme
+* for fields in ID registers", ARM DDI 0487C.a, any new
+* value is guaranteed to be higher than what we know already.
+* As a safe limit, we return the limit supported by the kernel.
+*/
+ default: return CONFIG_ARM64_PA_BITS;
+ }
+}
+
+static inline u32 id_aa64mmfr0_pa_range_bits(void)
+{
+ u64 mmfr0;
+
+ mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
+ return id_aa64mmfr0_parange_to_phys_shift(mmfr0 & 0x7);
+}
+
+static inline u32 id_aa64mmfr2_va_range_bits(void)
+{
+ u64 mmfr2;
+ u32 val;
+
+ mmfr2 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR2_EL1);
+ val = cpuid_feature_extract_unsigned_field(mmfr2,
+ ID_AA64MMFR2_LVA_SHIFT);
+ return ((val == ID_AA64MMFR2_VARANGE_52) ? VARANGE_52 : VARANGE_48);
+}
+
static inline bool system_supports_4kb_granule(void)
{
u64 mmfr0;
@@ -636,26 +675,6 @@ static inline void arm64_set_ssbd_mitigation(bool state) {}
extern int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
-static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange)
-{
- switch (parange) {
- case 0: return 32;
- case 1: return 36;
- case 2: return 40;
- case 3: return 42;
- case 4: return 44;
- case 5: return 48;
- case 6: return 52;
- /*
-* A future PE could use a value unknown to the kernel.
-* However, by the "D10.1.4 Principles of the ID scheme
-* for fields in ID registers", ARM DDI 0487C.a, any new
-* value is guaranteed to be higher than what we know already.
-* As a safe limit, we return the limit supported by the kernel.
-*/
- default: return CONFIG_ARM64_PA_BITS;
- }
-}
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 72dc4c011014..70910b14b2f3 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -617,9 +617,22 @@
#define ID_AA64MMFR0_TGRAN64_SUPPORTED 0x0
#define ID_AA64MMFR0_TGRAN16_NI0x0
#define ID_AA64MMFR0_TGRAN16_SUPPORTED 0x1
+#define ID_AA64MMFR0_PARANGE_320x0
+#define ID_AA64MMFR0_PARANGE_360x1
+#define ID_AA64MMFR0_PARANGE_400x2
+#define ID_AA64MMFR0_PARANGE_420x3
+#define ID_AA64MMFR0_PARANGE_440x4
#define ID_AA64MMFR0_PARANGE_480x5
#define ID_AA64MMFR0_PARANGE_520x6
+#define PARANGE_32 32
+#define PARANGE_36 36
+#define PARANGE_40 40
+#define PARANGE_42 42
+#define PARANGE_44 44
+#define PARANGE_48 48
+#define PARANGE_52 52
+
#ifdef CONFIG_ARM64_PA_BITS_52
#define ID_AA64MMFR0_PARANGE_MAX ID_AA64MMFR0_PARANGE_52
#else
@@ -646,6 +659,12 @@
#define ID_AA64MMFR2_UAO_SHIFT 4
#define ID_AA64MMFR2_CNP_SHIFT 0
+#define ID_AA64MMFR2_VARANGE_480x0
+#define ID_AA64MMFR2_VARANGE_520x1
+
+#define VARANGE_48 48
+#define VARANGE_52 52
+
/* id_aa64dfr0 */
#define ID_AA64DFR0_PMSV
[PATCH 2/2] arm64: Expose PARange via ID_AA64MMFR0_EL1 and VARange via ID_AA64MMFR2_EL1
ARMv8.2 architecture hardware extensions can support
upto 52-bit physical addresses (ARMv8.2-LPA) and 52-bit virtual
addresses (ARMv8.2-LVA).
User-space utilities like 'makedumpfile' can try and use the getauxval()
function to retrieve the underlying PARange and VARange values
supported.
An example implementation can be via the 'Appendix I: Example' shown
in 'Documentation/arm64/cpu-feature-registers.txt'. A reference
'makedumpfile' implementation which uses a similar approach is
available in [0].
So, we expose these properties via 'FTR_NONSTRICT' and 'FTR_VISIBLE'
settings for 'ID_AA64MMFR0_PARANGE_SHIFT' and 'ID_AA64MMFR2_LVA_SHIFT'.
[0].
https://github.com/bhupesh-sharma/makedumpfile/blob/9d7da4aad3efe79b448f48cc3454fcae46a316d6/arch/arm64.c#L499
Signed-off-by: Bhupesh Sharma
---
arch/arm64/kernel/cpufeature.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index f6d84e2c92fe..5cfc08cbf147 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -194,7 +194,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr0[] = {
* Differing PARange is fine as long as all peripherals and memory are
mapped
* within the minimum PARange of all CPUs
*/
- ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
ID_AA64MMFR0_PARANGE_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE,
ID_AA64MMFR0_PARANGE_SHIFT, 4, 0),
ARM64_FTR_END,
};
@@ -211,7 +211,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = {
static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64MMFR2_FWB_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64MMFR2_AT_SHIFT, 4, 0),
- ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64MMFR2_LVA_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_NONSTRICT, FTR_LOWER_SAFE,
ID_AA64MMFR2_LVA_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64MMFR2_IESB_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64MMFR2_LSM_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64MMFR2_UAO_SHIFT, 4, 0),
--
2.7.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 0/2] arm64: Expose physical and virtual address capabilities to user-space
With ARMv8.2-LVA and LPA architecture extensions, arm64 hardware which supports these extensions can support upto 52-bit virtual and 52-bit physical addresses respectively. Since at the moment we enable the support of these extensions via CONFIG flags, e.g. - LPA via CONFIG_ARM64_PA_BITS_52 there are no clear mechanisms in user-space right now to deteremine these CONFIG flag values and also determine the PARange and VARange address values. This patchset proposes two mechanisms to provide a user/user-space application more information on the same: 1. For a non-expert user, getting an idea about the virtual/physical address capabilities on an arm64 hardware via executing: $ cat /proc/cpuinfo can be quite useful as it provides a similar mechanism as available already on x86_64. PATCH 1/2 implements the same. 2. For a more involved user/user-space application, the Appendix I: Example in 'Documentation/arm64/cpu-feature-registers.txt' is pretty useful. If we can expose the PARange and VARange values via the MRS register reads to user-space, we can use them in user-space programs as a standard ABI. I already have a 'makedumpfile' user-space example that uses the same to determine ARMv8.2-LPA support. See [0]. PATCH 2/2 implements the same. [0]. https://github.com/bhupesh-sharma/makedumpfile/blob/9d7da4aad3efe79b448f48cc3454fcae46a316d6/arch/arm64.c#L499 Bhupesh Sharma (2): arm64: Expose address bits (physical/virtual) via cpuinfo arm64: Expose PARange via ID_AA64MMFR0_EL1 and VARange via ID_AA64MMFR2_EL1 arch/arm64/include/asm/cpufeature.h | 59 - arch/arm64/include/asm/sysreg.h | 19 arch/arm64/kernel/cpufeature.c | 4 +-- arch/arm64/kernel/cpuinfo.c | 4 ++- 4 files changed, 63 insertions(+), 23 deletions(-) -- 2.7.4 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH] makedumpfile: honor the CFLAGS from environment variables
On 1/28/2019 5:59 AM, Simon Horman wrote: > On Mon, Jan 28, 2019 at 06:50:45PM +0800, Kairui Song wrote: > > This make it easier for passing extra cflags, for example hardening > > flags could be passed in with enviroment variable. > > > > Signed-off-by: Kairui Song > > Thanks, I like this a lot. > > I would like to wake a little to see if there is review from > others before applying this. I like this, too, and sorry to steal this... :-) > > > --- > > Makefile | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 612b9d0..b511a78 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -8,11 +8,11 @@ ifeq ($(strip $CC),) > > CC = gcc > > endif > > > > -CFLAGS = -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ > > - -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' > > -CFLAGS_ARCH= -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > > +CFLAGS_ARCH = $(CFLAGS) -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > + -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE This expands the whole CFLAGS in advance and duplicates some flags, so we can use ":=" instead of "=" for CFLAGS_ARCH ? If this is fine, I'll fix it when merging. Thanks, Kazu > > +CFLAGS += -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > +-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ > > +-DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' > > # LDFLAGS = -L/usr/local/lib -I/usr/local/include > > > > HOST_ARCH := $(shell uname -m) > > -- > > 2.20.1 > > > > > > ___ > > kexec mailing list > > kexec@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/kexec > > ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH] makedumpfile: honor the CFLAGS from environment variables
On 1/28/2019 4:51 PM, Kazuhito Hagio wrote: > On 1/28/2019 5:59 AM, Simon Horman wrote: > > On Mon, Jan 28, 2019 at 06:50:45PM +0800, Kairui Song wrote: > > > This make it easier for passing extra cflags, for example hardening > > > flags could be passed in with enviroment variable. > > > > > > Signed-off-by: Kairui Song > > > > Thanks, I like this a lot. > > > > I would like to wake a little to see if there is review from > > others before applying this. > > I like this, too, and sorry to steal this... :-) > > > > > > --- > > > Makefile | 10 +- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/Makefile b/Makefile > > > index 612b9d0..b511a78 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -8,11 +8,11 @@ ifeq ($(strip $CC),) > > > CC = gcc > > > endif > > > > > > -CFLAGS = -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > > - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ > > > - -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' > > > -CFLAGS_ARCH = -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > > - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > > > +CFLAGS_ARCH = $(CFLAGS) -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > > + -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > > This expands the whole CFLAGS in advance and duplicates some flags, > so we can use ":=" instead of "=" for CFLAGS_ARCH ? > If this is fine, I'll fix it when merging. or it might be good to remove some redundant flags like this at this opportunity. CFLAGS_BASE := $(CFLAGS) -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE CFLAGS := $(CFLAGS_BASE) -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' CFLAGS_ARCH := $(CFLAGS_BASE) > > Thanks, > Kazu > > > > +CFLAGS += -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > > + -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ > > > + -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' > > > # LDFLAGS = -L/usr/local/lib -I/usr/local/include > > > > > > HOST_ARCH := $(shell uname -m) > > > -- > > > 2.20.1 > > > > > > > > > ___ > > > kexec mailing list > > > kexec@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/kexec > > > > > > > ___ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] makedumpfile: honor the CFLAGS from environment variables
On Tue, Jan 29, 2019 at 6:33 AM Kazuhito Hagio wrote: > > On 1/28/2019 4:51 PM, Kazuhito Hagio wrote: > > On 1/28/2019 5:59 AM, Simon Horman wrote: > > > On Mon, Jan 28, 2019 at 06:50:45PM +0800, Kairui Song wrote: > > > > This make it easier for passing extra cflags, for example hardening > > > > flags could be passed in with enviroment variable. > > > > > > > > Signed-off-by: Kairui Song > > > > > > Thanks, I like this a lot. > > > > > > I would like to wake a little to see if there is review from > > > others before applying this. > > > > I like this, too, and sorry to steal this... :-) > > > > > > > > > --- > > > > Makefile | 10 +- > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/Makefile b/Makefile > > > > index 612b9d0..b511a78 100644 > > > > --- a/Makefile > > > > +++ b/Makefile > > > > @@ -8,11 +8,11 @@ ifeq ($(strip $CC),) > > > > CC = gcc > > > > endif > > > > > > > > -CFLAGS = -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > > > - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ > > > > - -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' > > > > -CFLAGS_ARCH = -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > > > - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > > > > +CFLAGS_ARCH = $(CFLAGS) -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > > > > + -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > > > > This expands the whole CFLAGS in advance and duplicates some flags, > > so we can use ":=" instead of "=" for CFLAGS_ARCH ? > > If this is fine, I'll fix it when mergiKazuhito Hagio > > ng. > > or it might be good to remove some redundant flags like this > at this opportunity. > > CFLAGS_BASE := $(CFLAGS) -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ > -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > CFLAGS := $(CFLAGS_BASE) -DVERSION='"$(VERSION)"' > -DRELEASE_DATE='"$(DATE)"' > CFLAGS_ARCH := $(CFLAGS_BASE) Thanks for the suggestion, it is a good idea, simplifies the CFLAGS definition. Will send a V2 update. -- Best Regards, Kairui Song ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2] makedumpfile: honor the CFLAGS from environment variables
This makes it possible to pass in extra cflags, for example, hardening flags could be passed in with environment variable when building a hardened package. Also introduce a CFLAGS_BASE to hold common CFLAGS, which simplify the CFLAGS definition. Suggested-by: Kazuhito Hagio Signed-off-by: Kairui Song --- Update from V1: - Use a CFLAGS_BASE to simplify CFLAGS definition - Use immediate set rather than lazy set to avoid unexpected flag duplication Makefile | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 612b9d0..bca9984 100644 --- a/Makefile +++ b/Makefile @@ -8,11 +8,10 @@ ifeq ($(strip $CC),) CC = gcc endif -CFLAGS = -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ - -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' -CFLAGS_ARCH= -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ - -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE +CFLAGS_BASE := $(CFLAGS) -g -O2 -Wall -D_FILE_OFFSET_BITS=64 \ +-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE +CFLAGS := $(CFLAGS_BASE) -DVERSION='"$(VERSION)"' -DRELEASE_DATE='"$(DATE)"' +CFLAGS_ARCH := $(CFLAGS_BASE) # LDFLAGS = -L/usr/local/lib -I/usr/local/include HOST_ARCH := $(shell uname -m) -- 2.20.1 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr
On Fri, Jan 25, 2019 at 10:08 PM Borislav Petkov wrote: > > On Fri, Jan 25, 2019 at 09:45:18PM +0800, Dave Young wrote: > > AFAIK, some people prefer to explictly reserve crash memory at high > > region even if it is possible to reserve at low area. May because > > <4G memory is limited on large server, they want to leave this for other > > use. > > > > Yinghai or Vivek should know more about the history, probably they can > > recall some initial reason. > Go through the git log, and I found the initial introduction of crashkernel_high option. Refer to commit 55a20ee7804ab64ac90bcdd4e2868a42829e2784 Author: Yinghai Lu Date: Mon Apr 15 22:23:47 2013 -0700 x86, kdump: Retore crashkernel= to allocate under 896M Vivek found old kexec-tools does not work new kernel anymore. So change back crashkernel= back to old behavoir, and add crashkernel_high= to let user decide if buffer could be above 4G, and also new kexec-tools will be needed. But kexec-tools-2.0.3, released at 2012, can run 4.20 kernel with crashkernel=256M@5G, so I think only very old kexec-tools requires memory under 896M. Due to -1.few people running latest kernel with very old kexec-tools to date, -2. crashkernel=X is more popular than crashkernel=X.high, it should be time to eliminate this limit of crashkernel=X parameter, otherwise we will run into this bug. As for crashkernel=,high, I think it is a more professional option for who cares about the DMA32. On high-end machine, big reserved region is used for crashkernel(e.g. in this case 384M), which make the crowed situation under under 4GB memory worse. > Yes, just "prefer" is not good enough. There should be a technical > reason why that's there. > > Also, if the user doesn't care, then the code should be free to force > "high" and thus probe a different range for allocation. > Do you suggest to remove crashkernel=X,high parameter? Thanks, Pingfan > > Good question, still it may be some historical reason, but it is good to > > make them clear and rethink about it after long time. > > > > I also want to understand, need dig the log more. > > Good idea. That would be a very nice cleanup. :-) > > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv7] x86/kdump: bugfix, make the behavior of crashkernel=X consistent with kaslr
On Fri, Jan 25, 2019 at 6:39 PM Borislav Petkov wrote: > > > > Subject: Re: [PATCHv7] x86/kdump: bugfix, make the behavior of > > crashkernel=X > > s/bugfix, // > OK. > On Mon, Jan 21, 2019 at 01:16:08PM +0800, Pingfan Liu wrote: > > People reported crashkernel=384M reservation failed on a high end server > > with KASLR enabled. In that case there is enough free memory under 896M > > but crashkernel reservation still fails intermittently. > > > > The situation is crashkernel reservation code only finds free region under > > 896 MB with 128M aligned in case no ',high' being used. And KASLR could > > break the first 896M into several parts randomly thus the failure happens. > > This reads very strange. > What about " It turns out that crashkernel reservation code only tries to find a region under 896 MB, aligned on 128M. But KASLR randomly breaks big region inside [0,896M] into smaller pieces, not big enough as demanded in the "crashkernel=X" parameter." > > User has no way to predict and make sure crashkernel=xM working unless > > he/she use 'crashkernel=xM,high'. Since 'crashkernel=xM' is the most > > common use case this issue is a serious bug. > > > > And we can't answer questions raised from customer: > > 1) why it doesn't succeed to reserve 896 MB; > > 2) what's wrong with memory region under 4G; > > 3) why I have to add ',high', I only require 384 MB, not 3840 MB. > > Errr, this looks like communication issue. Sounds to me like the text > around crashkernel= in > What about dropping this section in commit log and another patch to fix the document? > Documentation/admin-guide/kernel-parameters.txt > > needs improving? > > > This patch tries to get memory region from 896 MB firstly, then [896MB,4G], > > Avoid having "This patch" or "This commit" in the commit message. It is > tautologically useless. > OK > Also, do > > $ git grep 'This patch' Documentation/process > > for more details. > > > finally above 4G. > > > > Dave Young sent the original post, and I just re-post it with commit log > > If he sent it, he should be the author I guess. > > > improvement as his requirement. > > http://lists.infradead.org/pipermail/kexec/2017-October/019571.html > > There was an old discussion below (previously posted by Chao Wang): > > https://lkml.org/lkml/2013/10/15/601 > > All that changelog info doesn't belong in the commit message ... > > > Signed-off-by: Pingfan Liu > > Cc: Dave Young > > Cc: Baoquan He > > Cc: Andrew Morton > > Cc: Mike Rapoport > > Cc: ying...@kernel.org, > > Cc: vgo...@redhat.com > > Cc: Randy Dunlap > > Cc: Borislav Petkov > > Cc: x...@kernel.org > > Cc: linux-ker...@vger.kernel.org > > --- > > but here. > > > v6 -> v7: commit log improvement > > arch/x86/kernel/setup.c | 16 > > 1 file changed, 16 insertions(+) > > > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > > index 3d872a5..fa62c81 100644 > > --- a/arch/x86/kernel/setup.c > > +++ b/arch/x86/kernel/setup.c > > @@ -551,6 +551,22 @@ static void __init reserve_crashkernel(void) > > high ? CRASH_ADDR_HIGH_MAX > >: CRASH_ADDR_LOW_MAX, > > crash_size, CRASH_ALIGN); > > +#ifdef CONFIG_X86_64 > > + /* > > + * crashkernel=X reserve below 896M fails? Try below 4G > > + */ > > + if (!high && !crash_base) > > + crash_base = memblock_find_in_range(CRASH_ALIGN, > > + (1ULL << 32), > > + crash_size, CRASH_ALIGN); > > + /* > > + * crashkernel=X reserve below 4G fails? Try MAXMEM > > + */ > > + if (!high && !crash_base) > > + crash_base = memblock_find_in_range(CRASH_ALIGN, > > + CRASH_ADDR_HIGH_MAX, > > + crash_size, CRASH_ALIGN); > > +#endif > > Ok, so this is silly: we know at which physical address KASLR allocated > the kernel so why aren't we querying that and seeing if there's enough > room before it or after it to call memblock_find_in_range() on the > bigger range? > Sorry, can not catch up with you. Do you suggestion memblock_find_in_range(0, kernel_start) and memblock_find_in_range(kernel_end, mem_end)? But the memory is truncated into fraction by many component which call memblock_reserve(), besides kernel. For the left question, Dave has follow the discussion in another email, will follow there. Thanks and regards, Pingfan > Also, why is "high" dealt with separately and why isn't the code > enforcing "high" if the normal reservation fails? > > The presence of high is requiring from our users to pay attention what > to use when the kernel can do all that automatically. Looks like a UI > fail to me. > > And look a
