Re: [PATCH 2/2 V2] Support kdump when AMD secure memory encryption is active

2018-06-19 Thread kbuild test robot
Hi Lianbo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17 next-20180614]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Lianbo-Jiang/Support-kdump-for-AMD-secure-memory-encryption-sme/20180614-164938
config: arm-sa1100 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   fs/proc/vmcore.o: In function `read_from_oldmem.part.0':
>> vmcore.c:(.text+0x1b4): undefined reference to `copy_oldmem_page_encrypted'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 2/2 V2] Support kdump when AMD secure memory encryption is active

2018-06-19 Thread kbuild test robot
Hi Lianbo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17 next-20180614]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Lianbo-Jiang/Support-kdump-for-AMD-secure-memory-encryption-sme/20180614-164938
config: i386-randconfig-c0-06141337 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   fs/proc/vmcore.o: In function `read_from_oldmem':
>> fs/proc/vmcore.c:127: undefined reference to `copy_oldmem_page_encrypted'

vim +127 fs/proc/vmcore.c

   100  
   101  /* Reads a page from the oldmem device from given offset. */
   102  static ssize_t read_from_oldmem(char *buf, size_t count,
   103  u64 *ppos, int userbuf,
   104  bool encrypted)
   105  {
   106  unsigned long pfn, offset;
   107  size_t nr_bytes;
   108  ssize_t read = 0, tmp;
   109  
   110  if (!count)
   111  return 0;
   112  
   113  offset = (unsigned long)(*ppos % PAGE_SIZE);
   114  pfn = (unsigned long)(*ppos / PAGE_SIZE);
   115  
   116  do {
   117  if (count > (PAGE_SIZE - offset))
   118  nr_bytes = PAGE_SIZE - offset;
   119  else
   120  nr_bytes = count;
   121  
   122  /* If pfn is not ram, return zeros for sparse dump 
files */
   123  if (pfn_is_ram(pfn) == 0)
   124  memset(buf, 0, nr_bytes);
   125  else {
   126  if (encrypted)
 > 127  tmp = copy_oldmem_page_encrypted(pfn, 
 > buf,
   128 nr_bytes, offset, 
userbuf);
   129  else
   130  tmp = copy_oldmem_page(pfn, buf, 
nr_bytes,
   131  offset, userbuf);
   132  
   133  if (tmp < 0)
   134  return tmp;
   135  }
   136  *ppos += nr_bytes;
   137  count -= nr_bytes;
   138  buf += nr_bytes;
   139  read += nr_bytes;
   140  ++pfn;
   141  offset = 0;
   142  } while (count);
   143  
   144  return read;
   145  }
   146  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 2/2 V2] Support kdump when AMD secure memory encryption is active

2018-06-15 Thread lijiang
在 2018年06月15日 15:19, Dave Young 写道:
> On 06/14/18 at 04:47pm, Lianbo Jiang wrote:
>> When sme enabled on AMD server, we also need to support kdump. Because
>> the memory is encrypted in the first kernel, we will remap the old memory
>> encrypted to the second kernel(crash kernel), and sme is also enabled in
>> the second kernel, otherwise the old memory encrypted can not be decrypted.
>> Because simply changing the value of a C-bit on a page will not
>> automatically encrypt the existing contents of a page, and any data in the
>> page prior to the C-bit modification will become unintelligible. A page of
>> memory that is marked encrypted will be automatically decrypted when read
>> from DRAM and will be automatically encrypted when written to DRAM.
>>
>> For the kdump, it is necessary to distinguish whether the memory is
>> encrypted. Furthermore, we should also know which part of the memory is
>> encrypted or decrypted. We will appropriately remap the memory according
>> to the specific situation in order to tell cpu how to deal with the data(
>> encrypted or unencrypted). For example, when sme enabled, if the old memory
>> is encrypted, we will remap the old memory in encrypted way, which will
>> automatically decrypt the old memory encrypted when we read those data from
>> the remapping address.
>>
>>  --
>> | first-kernel | second-kernel | kdump support |
>> |  (mem_encrypt=on|off)|   (yes|no)|
>> |--+---+---|
>> | on   | on| yes   |
>> | off  | off   | yes   |
>> | on   | off   | no|
>> | off  | on| no|
>> |__|___|___|
>>
>> Signed-off-by: Lianbo Jiang 
>> ---
>> Some changes based on V1:
>> 1. remove the '#ifdef' stuff throughout this patch.
>> 2. put some logic into the early_memremap_pgprot_adjust() and clean the
>> previous unnecessary changes, for example: arch/x86/include/asm/dmi.h,
>> arch/x86/kernel/acpi/boot.c, drivers/acpi/tables.c.
>> 3. rewrite two functions, copy_oldmem_page() and
>> copy_oldmem_page_encrypted().
>> 4. distingish sme_active() and sev_active(), when a distinction doesn't
>> need, mem_encrypt_active() will be used.
> 
> Lianbo, I think you modified this based on Tom's comment.
> But it would be good to add this only when you tested sev and it worked
> for you.
> 
Thank you, Dave.
That's a good advice, because the upstream kernel(host os) doesn't work in host
side, some drivers always go wrong, we can't test SEV for kdump patch. SEV'S 
code
will be removed in the patch V3. This patch is only for SME about kdump. As we
previously mentioned, maybe it is more reasonable to improve the SEV in another 
patch.

>> 5. clean compile warning in copy_device_table().
>>
>>  arch/x86/kernel/crash_dump_64.c | 42 
>> +++--
>>  arch/x86/mm/ioremap.c   |  4 
>>  drivers/iommu/amd_iommu_init.c  | 14 +-
> 
> Assume it will not break bisection it is better to split the iommu
> changes as one standalone patch and cc iommu list.
> 
>>  fs/proc/vmcore.c| 20 +++-
>>  include/linux/crash_dump.h  |  5 +
>>  kernel/kexec_core.c | 12 
> 
> Another two patches, one for kexec_core, another for vmcore.c will be
> better for review.
> 
>>  6 files changed, 81 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/x86/kernel/crash_dump_64.c 
>> b/arch/x86/kernel/crash_dump_64.c
>> index 4f2e077..a2c7b13 100644
>> --- a/arch/x86/kernel/crash_dump_64.c
>> +++ b/arch/x86/kernel/crash_dump_64.c
>> @@ -11,6 +11,23 @@
>>  #include 
>>  #include 
>>  
>> +static ssize_t copy_to(void *to, void *vaddr, unsigned long offset,
>> +   size_t size, int userbuf)
>> +{
>> +if (userbuf) {
>> +if (copy_to_user(to, vaddr + offset, size)) {
>> +iounmap(vaddr);
>> +return -ENOMEM;
>> +}
>> +} else
>> +memcpy(to, vaddr + offset, size);
>> +
>> +set_iounmap_nonlazy();
>> +iounmap(vaddr);
>> +
>> +return size;
>> +}
>> +
>>  /**
>>   * copy_oldmem_page - copy one page from "oldmem"
>>   * @pfn: page frame number to be copied
>> @@ -36,15 +53,20 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
>>  if (!vaddr)
>>  return -ENOMEM;
>>  
>> -if (userbuf) {
>> -if (copy_to_user(buf, vaddr + offset, csize)) {
>> -iounmap(vaddr);
>> -return -EFAULT;
>> -}
>> -} else
>> -memcpy(buf, vaddr + offset, csize);
>> +return copy_to(buf, vaddr, offset, csize, userbuf);
>> +}
>>  
>> -set_iounmap_nonlazy();
>> -iounmap(vaddr);
>> -return csize;
>> +ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf,
>> +size_t csize, unsigned long of

Re: [PATCH 2/2 V2] Support kdump when AMD secure memory encryption is active

2018-06-15 Thread Dave Young
On 06/14/18 at 04:56pm, Dave Young wrote:
> On 06/14/18 at 04:47pm, Lianbo Jiang wrote:
> > When sme enabled on AMD server, we also need to support kdump. Because
> > the memory is encrypted in the first kernel, we will remap the old memory
> > encrypted to the second kernel(crash kernel), and sme is also enabled in
> > the second kernel, otherwise the old memory encrypted can not be decrypted.
> > Because simply changing the value of a C-bit on a page will not
> > automatically encrypt the existing contents of a page, and any data in the
> > page prior to the C-bit modification will become unintelligible. A page of
> > memory that is marked encrypted will be automatically decrypted when read
> > from DRAM and will be automatically encrypted when written to DRAM.
> > 
> > For the kdump, it is necessary to distinguish whether the memory is
> > encrypted. Furthermore, we should also know which part of the memory is
> > encrypted or decrypted. We will appropriately remap the memory according
> > to the specific situation in order to tell cpu how to deal with the data(
> > encrypted or unencrypted). For example, when sme enabled, if the old memory
> > is encrypted, we will remap the old memory in encrypted way, which will
> > automatically decrypt the old memory encrypted when we read those data from
> > the remapping address.
> > 
> >  --
> > | first-kernel | second-kernel | kdump support |
> > |  (mem_encrypt=on|off)|   (yes|no)|
> > |--+---+---|
> > | on   | on| yes   |
> > | off  | off   | yes   |
> > | on   | off   | no|
> > | off  | on| no|
> > |__|___|___|
> > 
> > Signed-off-by: Lianbo Jiang 
> > ---
> > Some changes based on V1:
> > 1. remove the '#ifdef' stuff throughout this patch.
> > 2. put some logic into the early_memremap_pgprot_adjust() and clean the
> > previous unnecessary changes, for example: arch/x86/include/asm/dmi.h,
> > arch/x86/kernel/acpi/boot.c, drivers/acpi/tables.c.
> > 3. rewrite two functions, copy_oldmem_page() and
> > copy_oldmem_page_encrypted().
> > 4. distingish sme_active() and sev_active(), when a distinction doesn't
> > need, mem_encrypt_active() will be used.
> > 5. clean compile warning in copy_device_table().
> > 
> >  arch/x86/kernel/crash_dump_64.c | 42 
> > +++--
> >  arch/x86/mm/ioremap.c   |  4 
> >  drivers/iommu/amd_iommu_init.c  | 14 +-
> >  fs/proc/vmcore.c| 20 +++-
> >  include/linux/crash_dump.h  |  5 +
> >  kernel/kexec_core.c | 12 
> >  6 files changed, 81 insertions(+), 16 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/crash_dump_64.c 
> > b/arch/x86/kernel/crash_dump_64.c
> > index 4f2e077..a2c7b13 100644
> > --- a/arch/x86/kernel/crash_dump_64.c
> > +++ b/arch/x86/kernel/crash_dump_64.c
> > @@ -11,6 +11,23 @@
> >  #include 
> >  #include 
> >  
> > +static ssize_t copy_to(void *to, void *vaddr, unsigned long offset,
> > +  size_t size, int userbuf)
> > +{
> > +   if (userbuf) {
> > +   if (copy_to_user(to, vaddr + offset, size)) {
> > +   iounmap(vaddr);
> > +   return -ENOMEM;
> > +   }
> > +   } else
> > +   memcpy(to, vaddr + offset, size);
> > +
> > +   set_iounmap_nonlazy();
> > +   iounmap(vaddr);
> > +
> > +   return size;
> > +}
> 
> Hmm, the function name copy_to is strange
> 
> Also since iounmap is needed in the code path but not paired with
> ioremap, it is bad.  If you really want this function then need moving
> the iounmap related code to caller function.  And better to rename this
> function as eg. copy_oldmem()
> 

Rechecking the comments, and the robot reported build error, it can be
like this:

* move the #define copy_oldmem_page_encrypted in header file to
  use a dummy inline function in case without the config option enabled.

* conditional compile your new function in Makefile with a new .c for
  your copy_oldmem_page_encrypted

> > +
> >  /**
> >   * copy_oldmem_page - copy one page from "oldmem"
> >   * @pfn: page frame number to be copied
> > @@ -36,15 +53,20 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
> > if (!vaddr)
> > return -ENOMEM;
> >  
> > -   if (userbuf) {
> > -   if (copy_to_user(buf, vaddr + offset, csize)) {
> > -   iounmap(vaddr);
> > -   return -EFAULT;
> > -   }
> > -   } else
> > -   memcpy(buf, vaddr + offset, csize);
> > +   return copy_to(buf, vaddr, offset, csize, userbuf);
> > +}
> >  
> > -   set_iounmap_nonlazy();
> > -   iounmap(vaddr);
> > -   return csize;
> > +ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf,
> > +   size_t csize, unsigned long offset, int user

Re: [PATCH 2/2 V2] Support kdump when AMD secure memory encryption is active

2018-06-15 Thread Dave Young
On 06/14/18 at 04:47pm, Lianbo Jiang wrote:
> When sme enabled on AMD server, we also need to support kdump. Because
> the memory is encrypted in the first kernel, we will remap the old memory
> encrypted to the second kernel(crash kernel), and sme is also enabled in
> the second kernel, otherwise the old memory encrypted can not be decrypted.
> Because simply changing the value of a C-bit on a page will not
> automatically encrypt the existing contents of a page, and any data in the
> page prior to the C-bit modification will become unintelligible. A page of
> memory that is marked encrypted will be automatically decrypted when read
> from DRAM and will be automatically encrypted when written to DRAM.
> 
> For the kdump, it is necessary to distinguish whether the memory is
> encrypted. Furthermore, we should also know which part of the memory is
> encrypted or decrypted. We will appropriately remap the memory according
> to the specific situation in order to tell cpu how to deal with the data(
> encrypted or unencrypted). For example, when sme enabled, if the old memory
> is encrypted, we will remap the old memory in encrypted way, which will
> automatically decrypt the old memory encrypted when we read those data from
> the remapping address.
> 
>  --
> | first-kernel | second-kernel | kdump support |
> |  (mem_encrypt=on|off)|   (yes|no)|
> |--+---+---|
> | on   | on| yes   |
> | off  | off   | yes   |
> | on   | off   | no|
> | off  | on| no|
> |__|___|___|
> 
> Signed-off-by: Lianbo Jiang 
> ---
> Some changes based on V1:
> 1. remove the '#ifdef' stuff throughout this patch.
> 2. put some logic into the early_memremap_pgprot_adjust() and clean the
> previous unnecessary changes, for example: arch/x86/include/asm/dmi.h,
> arch/x86/kernel/acpi/boot.c, drivers/acpi/tables.c.
> 3. rewrite two functions, copy_oldmem_page() and
> copy_oldmem_page_encrypted().
> 4. distingish sme_active() and sev_active(), when a distinction doesn't
> need, mem_encrypt_active() will be used.

Lianbo, I think you modified this based on Tom's comment.
But it would be good to add this only when you tested sev and it worked
for you.

> 5. clean compile warning in copy_device_table().
> 
>  arch/x86/kernel/crash_dump_64.c | 42 
> +++--
>  arch/x86/mm/ioremap.c   |  4 
>  drivers/iommu/amd_iommu_init.c  | 14 +-

Assume it will not break bisection it is better to split the iommu
changes as one standalone patch and cc iommu list.

>  fs/proc/vmcore.c| 20 +++-
>  include/linux/crash_dump.h  |  5 +
>  kernel/kexec_core.c | 12 

Another two patches, one for kexec_core, another for vmcore.c will be
better for review.

>  6 files changed, 81 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
> index 4f2e077..a2c7b13 100644
> --- a/arch/x86/kernel/crash_dump_64.c
> +++ b/arch/x86/kernel/crash_dump_64.c
> @@ -11,6 +11,23 @@
>  #include 
>  #include 
>  
> +static ssize_t copy_to(void *to, void *vaddr, unsigned long offset,
> +size_t size, int userbuf)
> +{
> + if (userbuf) {
> + if (copy_to_user(to, vaddr + offset, size)) {
> + iounmap(vaddr);
> + return -ENOMEM;
> + }
> + } else
> + memcpy(to, vaddr + offset, size);
> +
> + set_iounmap_nonlazy();
> + iounmap(vaddr);
> +
> + return size;
> +}
> +
>  /**
>   * copy_oldmem_page - copy one page from "oldmem"
>   * @pfn: page frame number to be copied
> @@ -36,15 +53,20 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
>   if (!vaddr)
>   return -ENOMEM;
>  
> - if (userbuf) {
> - if (copy_to_user(buf, vaddr + offset, csize)) {
> - iounmap(vaddr);
> - return -EFAULT;
> - }
> - } else
> - memcpy(buf, vaddr + offset, csize);
> + return copy_to(buf, vaddr, offset, csize, userbuf);
> +}
>  
> - set_iounmap_nonlazy();
> - iounmap(vaddr);
> - return csize;
> +ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf,
> + size_t csize, unsigned long offset, int userbuf)
> +{
> + void  *vaddr;
> +
> + if (!csize)
> + return 0;
> +
> + vaddr = ioremap_encrypted(pfn << PAGE_SHIFT, PAGE_SIZE);
> + if (!vaddr)
> + return -ENOMEM;
> +
> + return copy_to(buf, vaddr, offset, csize, userbuf);
>  }
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 24e0920..e365fc4 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 

Re: [PATCH 2/2 V2] Support kdump when AMD secure memory encryption is active

2018-06-14 Thread kbuild test robot
Hi Lianbo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.17 next-20180614]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Lianbo-Jiang/Support-kdump-for-AMD-secure-memory-encryption-sme/20180614-164938
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


vim +904 drivers/iommu/amd_iommu_init.c

   854  
   855  
   856  static bool copy_device_table(void)
   857  {
   858  u64 int_ctl, int_tab_len, entry = 0, last_entry = 0;
   859  struct dev_table_entry *old_devtb = NULL;
   860  u32 lo, hi, devid, old_devtb_size;
   861  phys_addr_t old_devtb_phys;
   862  struct amd_iommu *iommu;
   863  u16 dom_id, dte_v, irq_v;
   864  gfp_t gfp_flag;
   865  u64 tmp;
   866  
   867  if (!amd_iommu_pre_enabled)
   868  return false;
   869  
   870  pr_warn("Translation is already enabled - trying to copy 
translation structures\n");
   871  for_each_iommu(iommu) {
   872  /* All IOMMUs should use the same device table with the 
same size */
   873  lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
   874  hi = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET + 
4);
   875  entry = (((u64) hi) << 32) + lo;
   876  if (last_entry && last_entry != entry) {
   877  pr_err("IOMMU:%d should use the same dev table 
as others!\n",
   878  iommu->index);
   879  return false;
   880  }
   881  last_entry = entry;
   882  
   883  old_devtb_size = ((entry & ~PAGE_MASK) + 1) << 12;
   884  if (old_devtb_size != dev_table_size) {
   885  pr_err("The device table size of IOMMU:%d is 
not expected!\n",
   886  iommu->index);
   887  return false;
   888  }
   889  }
   890  
   891  old_devtb_phys = entry & PAGE_MASK;
   892  /*
   893   *  When sme enable in the first kernel, old_devtb_phys 
includes the
   894   *  memory encryption mask(sme_me_mask), we must remove the 
memory
   895   *  encryption mask to obtain the true physical address in 
kdump mode.
   896   */
   897  if (mem_encrypt_active() && is_kdump_kernel())
   898  old_devtb_phys = __sme_clr(old_devtb_phys);
   899  if (old_devtb_phys >= 0x1ULL) {
   900  pr_err("The address of old device table is above 4G, 
not trustworthy!\n");
   901  return false;
   902  }
   903  if (mem_encrypt_active() && is_kdump_kernel())
 > 904  old_devtb = (void *)ioremap_encrypted(old_devtb_phys,
   905   dev_table_size);
   906  else
   907  old_devtb = memremap(old_devtb_phys,
   908  dev_table_size, MEMREMAP_WB);
   909  if (!old_devtb)
   910  return false;
   911  
   912  gfp_flag = GFP_KERNEL | __GFP_ZERO | GFP_DMA32;
   913  old_dev_tbl_cpy = (void *)__get_free_pages(gfp_flag,
   914  get_order(dev_table_size));
   915  if (old_dev_tbl_cpy == NULL) {
   916  pr_err("Failed to allocate memory for copying old 
device table!\n");
   917  return false;
   918  }
   919  
   920  for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
   921  old_dev_tbl_cpy[devid] = old_devtb[devid];
   922  dom_id = old_devtb[devid].data[1] & DEV_DOMID_MASK;
   923  dte_v = old_devtb[devid].data[0] & DTE_FLAG_V;
   924  
   925  if (dte_v && dom_id) {
   926  old_dev_tbl_cpy[devid].data[0] = 
old_devtb[devid].data[0];
   927  old_dev_tbl_cpy[devid].data[1] = 
old_devtb[devid].data[1];
   928  __set_bit(dom_id, amd_iommu_pd_alloc_bitmap);
   929  /* If gcr3 table existed, mask it out */
   930  if (old_devtb[devid].data[0] & DTE_FLAG_GV) {
   931  tmp = DTE_GCR3_VAL_B(~0ULL) << 
DTE_GCR3_SHIFT_B;
   932  tmp |= DTE_GCR3_VAL_C(~0ULL) << 
DTE_GCR3_SHIFT_C;
   933  old_dev_tbl_cpy[devid].data[1] &= ~tmp;
   934  tmp = DTE_GCR3_VAL_A(~0ULL) << 
DTE_GCR3_SHIFT_A;
   935  tmp |= DTE_FLAG_GV;
   936   

Re: [PATCH 2/2 V2] Support kdump when AMD secure memory encryption is active

2018-06-14 Thread lijiang
在 2018年06月14日 20:55, kbuild test robot 写道:
> Hi Lianbo,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.17 next-20180614]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Lianbo-Jiang/Support-kdump-for-AMD-secure-memory-encryption-sme/20180614-164938
> config: i386-randconfig-c0-06141337 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>fs/proc/vmcore.o: In function `read_from_oldmem':
>>> fs/proc/vmcore.c:127: undefined reference to `copy_oldmem_page_encrypted'
> 
It is strange, it doesn't have this error in my test. I will look for another 
environment to test
and see whether it can be reproduced.
Maybe the compile error will be fixed in the patch V3.

Thanks.
Lianbo 
> vim +127 fs/proc/vmcore.c
> 
>100
>101/* Reads a page from the oldmem device from given offset. */
>102static ssize_t read_from_oldmem(char *buf, size_t count,
>103u64 *ppos, int userbuf,
>104bool encrypted)
>105{
>106unsigned long pfn, offset;
>107size_t nr_bytes;
>108ssize_t read = 0, tmp;
>109
>110if (!count)
>111return 0;
>112
>113offset = (unsigned long)(*ppos % PAGE_SIZE);
>114pfn = (unsigned long)(*ppos / PAGE_SIZE);
>115
>116do {
>117if (count > (PAGE_SIZE - offset))
>118nr_bytes = PAGE_SIZE - offset;
>119else
>120nr_bytes = count;
>121
>122/* If pfn is not ram, return zeros for sparse 
> dump files */
>123if (pfn_is_ram(pfn) == 0)
>124memset(buf, 0, nr_bytes);
>125else {
>126if (encrypted)
>  > 127tmp = 
> copy_oldmem_page_encrypted(pfn, buf,
>128   nr_bytes, 
> offset, userbuf);
>129else
>130tmp = copy_oldmem_page(pfn, 
> buf, nr_bytes,
>131offset, 
> userbuf);
>132
>133if (tmp < 0)
>134return tmp;
>135}
>136*ppos += nr_bytes;
>137count -= nr_bytes;
>138buf += nr_bytes;
>139read += nr_bytes;
>140++pfn;
>141offset = 0;
>142} while (count);
>143
>144return read;
>145}
>146
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 2/2 V2] Support kdump when AMD secure memory encryption is active

2018-06-14 Thread Dave Young
On 06/14/18 at 04:47pm, Lianbo Jiang wrote:
> When sme enabled on AMD server, we also need to support kdump. Because
> the memory is encrypted in the first kernel, we will remap the old memory
> encrypted to the second kernel(crash kernel), and sme is also enabled in
> the second kernel, otherwise the old memory encrypted can not be decrypted.
> Because simply changing the value of a C-bit on a page will not
> automatically encrypt the existing contents of a page, and any data in the
> page prior to the C-bit modification will become unintelligible. A page of
> memory that is marked encrypted will be automatically decrypted when read
> from DRAM and will be automatically encrypted when written to DRAM.
> 
> For the kdump, it is necessary to distinguish whether the memory is
> encrypted. Furthermore, we should also know which part of the memory is
> encrypted or decrypted. We will appropriately remap the memory according
> to the specific situation in order to tell cpu how to deal with the data(
> encrypted or unencrypted). For example, when sme enabled, if the old memory
> is encrypted, we will remap the old memory in encrypted way, which will
> automatically decrypt the old memory encrypted when we read those data from
> the remapping address.
> 
>  --
> | first-kernel | second-kernel | kdump support |
> |  (mem_encrypt=on|off)|   (yes|no)|
> |--+---+---|
> | on   | on| yes   |
> | off  | off   | yes   |
> | on   | off   | no|
> | off  | on| no|
> |__|___|___|
> 
> Signed-off-by: Lianbo Jiang 
> ---
> Some changes based on V1:
> 1. remove the '#ifdef' stuff throughout this patch.
> 2. put some logic into the early_memremap_pgprot_adjust() and clean the
> previous unnecessary changes, for example: arch/x86/include/asm/dmi.h,
> arch/x86/kernel/acpi/boot.c, drivers/acpi/tables.c.
> 3. rewrite two functions, copy_oldmem_page() and
> copy_oldmem_page_encrypted().
> 4. distingish sme_active() and sev_active(), when a distinction doesn't
> need, mem_encrypt_active() will be used.
> 5. clean compile warning in copy_device_table().
> 
>  arch/x86/kernel/crash_dump_64.c | 42 
> +++--
>  arch/x86/mm/ioremap.c   |  4 
>  drivers/iommu/amd_iommu_init.c  | 14 +-
>  fs/proc/vmcore.c| 20 +++-
>  include/linux/crash_dump.h  |  5 +
>  kernel/kexec_core.c | 12 
>  6 files changed, 81 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
> index 4f2e077..a2c7b13 100644
> --- a/arch/x86/kernel/crash_dump_64.c
> +++ b/arch/x86/kernel/crash_dump_64.c
> @@ -11,6 +11,23 @@
>  #include 
>  #include 
>  
> +static ssize_t copy_to(void *to, void *vaddr, unsigned long offset,
> +size_t size, int userbuf)
> +{
> + if (userbuf) {
> + if (copy_to_user(to, vaddr + offset, size)) {
> + iounmap(vaddr);
> + return -ENOMEM;
> + }
> + } else
> + memcpy(to, vaddr + offset, size);
> +
> + set_iounmap_nonlazy();
> + iounmap(vaddr);
> +
> + return size;
> +}

Hmm, the function name copy_to is strange

Also since iounmap is needed in the code path but not paired with
ioremap, it is bad.  If you really want this function then need moving
the iounmap related code to caller function.  And better to rename this
function as eg. copy_oldmem()

> +
>  /**
>   * copy_oldmem_page - copy one page from "oldmem"
>   * @pfn: page frame number to be copied
> @@ -36,15 +53,20 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
>   if (!vaddr)
>   return -ENOMEM;
>  
> - if (userbuf) {
> - if (copy_to_user(buf, vaddr + offset, csize)) {
> - iounmap(vaddr);
> - return -EFAULT;
> - }
> - } else
> - memcpy(buf, vaddr + offset, csize);
> + return copy_to(buf, vaddr, offset, csize, userbuf);
> +}
>  
> - set_iounmap_nonlazy();
> - iounmap(vaddr);
> - return csize;
> +ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf,
> + size_t csize, unsigned long offset, int userbuf)
> +{
> + void  *vaddr;
> +
> + if (!csize)
> + return 0;
> +
> + vaddr = ioremap_encrypted(pfn << PAGE_SHIFT, PAGE_SIZE);
> + if (!vaddr)
> + return -ENOMEM;
> +
> + return copy_to(buf, vaddr, offset, csize, userbuf);
>  }
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 24e0920..e365fc4 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "physaddr.h"
>  
> @@ -69

[PATCH 2/2 V2] Support kdump when AMD secure memory encryption is active

2018-06-14 Thread Lianbo Jiang
When sme enabled on AMD server, we also need to support kdump. Because
the memory is encrypted in the first kernel, we will remap the old memory
encrypted to the second kernel(crash kernel), and sme is also enabled in
the second kernel, otherwise the old memory encrypted can not be decrypted.
Because simply changing the value of a C-bit on a page will not
automatically encrypt the existing contents of a page, and any data in the
page prior to the C-bit modification will become unintelligible. A page of
memory that is marked encrypted will be automatically decrypted when read
from DRAM and will be automatically encrypted when written to DRAM.

For the kdump, it is necessary to distinguish whether the memory is
encrypted. Furthermore, we should also know which part of the memory is
encrypted or decrypted. We will appropriately remap the memory according
to the specific situation in order to tell cpu how to deal with the data(
encrypted or unencrypted). For example, when sme enabled, if the old memory
is encrypted, we will remap the old memory in encrypted way, which will
automatically decrypt the old memory encrypted when we read those data from
the remapping address.

 --
| first-kernel | second-kernel | kdump support |
|  (mem_encrypt=on|off)|   (yes|no)|
|--+---+---|
| on   | on| yes   |
| off  | off   | yes   |
| on   | off   | no|
| off  | on| no|
|__|___|___|

Signed-off-by: Lianbo Jiang 
---
Some changes based on V1:
1. remove the '#ifdef' stuff throughout this patch.
2. put some logic into the early_memremap_pgprot_adjust() and clean the
previous unnecessary changes, for example: arch/x86/include/asm/dmi.h,
arch/x86/kernel/acpi/boot.c, drivers/acpi/tables.c.
3. rewrite two functions, copy_oldmem_page() and
copy_oldmem_page_encrypted().
4. distingish sme_active() and sev_active(), when a distinction doesn't
need, mem_encrypt_active() will be used.
5. clean compile warning in copy_device_table().

 arch/x86/kernel/crash_dump_64.c | 42 +++--
 arch/x86/mm/ioremap.c   |  4 
 drivers/iommu/amd_iommu_init.c  | 14 +-
 fs/proc/vmcore.c| 20 +++-
 include/linux/crash_dump.h  |  5 +
 kernel/kexec_core.c | 12 
 6 files changed, 81 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
index 4f2e077..a2c7b13 100644
--- a/arch/x86/kernel/crash_dump_64.c
+++ b/arch/x86/kernel/crash_dump_64.c
@@ -11,6 +11,23 @@
 #include 
 #include 
 
+static ssize_t copy_to(void *to, void *vaddr, unsigned long offset,
+  size_t size, int userbuf)
+{
+   if (userbuf) {
+   if (copy_to_user(to, vaddr + offset, size)) {
+   iounmap(vaddr);
+   return -ENOMEM;
+   }
+   } else
+   memcpy(to, vaddr + offset, size);
+
+   set_iounmap_nonlazy();
+   iounmap(vaddr);
+
+   return size;
+}
+
 /**
  * copy_oldmem_page - copy one page from "oldmem"
  * @pfn: page frame number to be copied
@@ -36,15 +53,20 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
if (!vaddr)
return -ENOMEM;
 
-   if (userbuf) {
-   if (copy_to_user(buf, vaddr + offset, csize)) {
-   iounmap(vaddr);
-   return -EFAULT;
-   }
-   } else
-   memcpy(buf, vaddr + offset, csize);
+   return copy_to(buf, vaddr, offset, csize, userbuf);
+}
 
-   set_iounmap_nonlazy();
-   iounmap(vaddr);
-   return csize;
+ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf,
+   size_t csize, unsigned long offset, int userbuf)
+{
+   void  *vaddr;
+
+   if (!csize)
+   return 0;
+
+   vaddr = ioremap_encrypted(pfn << PAGE_SHIFT, PAGE_SIZE);
+   if (!vaddr)
+   return -ENOMEM;
+
+   return copy_to(buf, vaddr, offset, csize, userbuf);
 }
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 24e0920..e365fc4 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "physaddr.h"
 
@@ -696,6 +697,9 @@ pgprot_t __init 
early_memremap_pgprot_adjust(resource_size_t phys_addr,
if (encrypted_prot && memremap_should_map_decrypted(phys_addr, size))
encrypted_prot = false;
 
+   if (sme_active() && is_kdump_kernel())
+   encrypted_prot = false;
+
return encrypted_prot ? pgprot_encrypted(prot)
  : pgprot_decrypted(prot);
 }
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 904c575..5e535a6 1