Re: [PATCH kernel v2 1/2] vfio/spapr: Use IOMMU pageshift rather than pagesize
On Tue, 26 Jun 2018 15:59:25 +1000
Alexey Kardashevskiy wrote:
> The size is always equal to 1 page so let's use this. Later on this will
> be used for other checks which use page shifts to check the granularity
> of access.
>
> This should cause no behavioral change.
>
> Reviewed-by: David Gibson
> Signed-off-by: Alexey Kardashevskiy
> ---
> drivers/vfio/vfio_iommu_spapr_tce.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
I assume a v3+ will go in through the ppc tree since the bulk of the
series is there. For this,
Acked-by: Alex Williamson
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c
> b/drivers/vfio/vfio_iommu_spapr_tce.c
> index 759a5bd..2da5f05 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -457,13 +457,13 @@ static void tce_iommu_unuse_page(struct tce_container
> *container,
> }
>
> static int tce_iommu_prereg_ua_to_hpa(struct tce_container *container,
> - unsigned long tce, unsigned long size,
> + unsigned long tce, unsigned long shift,
> unsigned long *phpa, struct mm_iommu_table_group_mem_t **pmem)
> {
> long ret = 0;
> struct mm_iommu_table_group_mem_t *mem;
>
> - mem = mm_iommu_lookup(container->mm, tce, size);
> + mem = mm_iommu_lookup(container->mm, tce, 1ULL << shift);
> if (!mem)
> return -EINVAL;
>
> @@ -487,7 +487,7 @@ static void tce_iommu_unuse_page_v2(struct tce_container
> *container,
> if (!pua)
> return;
>
> - ret = tce_iommu_prereg_ua_to_hpa(container, *pua, IOMMU_PAGE_SIZE(tbl),
> + ret = tce_iommu_prereg_ua_to_hpa(container, *pua, tbl->it_page_shift,
> &hpa, &mem);
> if (ret)
> pr_debug("%s: tce %lx at #%lx was not cached, ret=%d\n",
> @@ -611,7 +611,7 @@ static long tce_iommu_build_v2(struct tce_container
> *container,
> entry + i);
>
> ret = tce_iommu_prereg_ua_to_hpa(container,
> - tce, IOMMU_PAGE_SIZE(tbl), &hpa, &mem);
> + tce, tbl->it_page_shift, &hpa, &mem);
> if (ret)
> break;
>
Re: [PATCH 1/2] powerpc/pkeys: preallocate execute_only key only if the key is available.
On Fri, Jun 29, 2018 at 09:58:37PM -0300, Thiago Jung Bauermann wrote:
>
> Gabriel Paubert writes:
>
> > On Thu, Jun 28, 2018 at 11:56:34PM -0300, Thiago Jung Bauermann wrote:
> >>
> >> Hello,
> >>
> >> Ram Pai writes:
> >>
> >> > Key 2 is preallocated and reserved for execute-only key. In rare
> >> > cases if key-2 is unavailable, mprotect(PROT_EXEC) will behave
> >> > incorrectly. NOTE: mprotect(PROT_EXEC) uses execute-only key.
> >> >
> >> > Ensure key 2 is available for preallocation before reserving it for
> >> > execute_only purpose. Problem noticed by Michael Ellermen.
> >>
> >> Since "powerpc/pkeys: Preallocate execute-only key" isn't upstream yet,
> >> this patch could be squashed into it.
> >>
> >> > Signed-off-by: Ram Pai
> >> > ---
> >> > arch/powerpc/mm/pkeys.c | 14 +-
> >> > 1 files changed, 9 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> >> > index cec990c..0b03914 100644
> >> > --- a/arch/powerpc/mm/pkeys.c
> >> > +++ b/arch/powerpc/mm/pkeys.c
> >> > @@ -19,6 +19,7 @@
> >> > u64 pkey_amr_mask; /* Bits in AMR not to be touched */
> >> > u64 pkey_iamr_mask;/* Bits in AMR not to be touched */
> >> > u64 pkey_uamor_mask; /* Bits in UMOR not to be touched */
> >> > +int execute_only_key = 2;
> >> >
> >> > #define AMR_BITS_PER_PKEY 2
> >> > #define AMR_RD_BIT 0x1UL
> >> > @@ -26,7 +27,6 @@
> >> > #define IAMR_EX_BIT 0x1UL
> >> > #define PKEY_REG_BITS (sizeof(u64)*8)
> >> > #define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))
> >> > -#define EXECUTE_ONLY_KEY 2
> >> >
> >> > static void scan_pkey_feature(void)
> >> > {
> >> > @@ -122,8 +122,12 @@ int pkey_initialize(void)
> >> > #else
> >> > os_reserved = 0;
> >> > #endif
> >> > +
> >> > +if ((pkeys_total - os_reserved) <= execute_only_key)
> >> > +execute_only_key = -1;
> >> > +
> >> > /* Bits are in LE format. */
> >> > -reserved_allocation_mask = (0x1 << 1) | (0x1 <<
> >> > EXECUTE_ONLY_KEY);
> >> > +reserved_allocation_mask = (0x1 << 1) | (0x1 <<
> >> > execute_only_key);
> >>
> >> My understanding is that left-shifting by a negative amount is undefined
> >> behavior in C. A quick test tells me that at least on the couple of
> >> machines I tested, 1 < -1 = 0. Does GCC guarantee that behavior?
> >
> > Not in general. It probably always works on Power because of the definition
> > of the machine instruction for shifts with variable amount (consider the
> > shift amount unsigned and take it modulo twice the width of the operand),
>
> Ok, thanks for confirming.
>
> > but for example it fails on x86 (1<<-1 gives 0x8000).
>
> Strange, this works on my laptop with an Intel(R) Core(TM) i5-7300U CPU:
>
> $ cat blah.c
> #include
>
> int main(int argc, char *argv[])
> {
> printf("1 << -1 = %llx\n", (unsigned long long) 1 << -1);
> return 0;
> }
> $ make blah
> cc blah.c -o blah
> blah.c: In function 'main':
> blah.c:5:52: warning: left shift count is negative [-Wshift-count-negative]
> printf("1 << -1 = %llx\n", (unsigned long long) 1 << -1);
> ^~
> $ ./blah
> 1 << -1 = 0
>
> Even if I change the cast and printf format to int, the result is the
> same. Or am I doing it wrong?
Try something more dynamic, 1 << -1 is evaluated at compile time by gcc,
and when you get a warning, the compile time expression evaluation does
not always give the same result as the run time machine instructions.
To test this I wrote (yes, the error checking is approximate, it would
be better to use strtol):
/***/
#include
#include
int main(int argc, char **argv)
{
int val, amount, valid;
if (argc != 3) {
printf("Needs 2 arguments!\n");
return EXIT_FAILURE;
}
valid = sscanf(argv[1], "%d", &val);
if (valid == 1) {
valid = sscanf(argv[2], "%d", &amount);
}
if (valid == 1) {
printf("%d shifted by %d is %d\n", val, amount, val<
> >> If so, a comment pointing this out would make this less confusing.
> >
> > Unless I miss something, this code is run once at boot, so its
> > performance is irrelevant.
> >
> > In this case simply rewrite it as:
> >
> > reserved_allocation_mask = 0x1 << 1;
> > if ( (pkeys_total - os_reserved) <= execute_only_key) {
> > execute_only_key = -1;
> > } else {
> > reserved_allocation_mask = (0x1 << 1) | (0x1 <<
> > execute_only_key);
> > }
>
> I agree it's clearer and more robust code (except that the first
> line should be inside the if block).
Indeed, sorry for this.
Gabriel
>
> >
> > Caveat, I have assumed that the code will either:
> > - only run once,
> > - pkeys_total and os_reserved are int, not unsigned
>
> B
Re: [Update] Regression in 4.18 - 32-bit PowerPC crashes on boot - bisected to commit 1d40a5ea01d5
On 06/30/2018 04:31 AM, christophe leroy wrote:
Le 29/06/2018 à 22:42, Larry Finger a écrit :
My PowerBook G4 Aluminum crashes on boot with 4.18-rcX kernels with a kernel
BUG at include/linux/page-flags.h:700! The problem was bisected to commit
1d40a5ea01d5 ("mm: mark pages in use for page tables"). It is not possible to
capture the bug with anything other than a camera. The first few lines of the
traceback are as follows:
free_pgd_range+0x19c/0x30c (unreliable)
free_pgtables_0xa0/0xb0
exit_pmap+0xf4/0x16c
mmput+0x64/0xf0
do_exit+0x33c/0x89c
oops_end+0x13c/0x144
_exception_pkey+0x58/0x128
ret_from_except_full+0x0/0x4
--- interrupt: 700 at free_pgd_range+0x19c/0x30c
LR = free_pgd_range+0x19c/0x30c
free_pgtables+0xa/0xb
exit_mnap+0xf4/0x16c
mmput+0x64/0xf0
flush_old_exec+0x490/0x550
I have more information regarding this BUG. Line 700 of page-flags.h is the
macro PAGE_TYPE_OPS(Table, table). For further debugging, I manually expanded
the macro, and found that the bug line is VM_BUG_ON_PAGE(!PageTable(page),
page) in routine __ClearPageTable(), which is called from pgtable_page_dtor()
in include/linux/mm.h. I also added a printk call to PageTable() that logs
page->page_type. The routine was called twice. The first had page_type of
0xfbff, which would have been expected for a . The second call had
0x, which led to the BUG.
Oh, seems to be the one I noticed and told Aneesh about
(https://patchwork.ozlabs.org/patch/922771/)
Aneesh provided the patch https://patchwork.ozlabs.org/patch/934111/ for it,
does it help ?
Yes, those changes fix the problem.
Larry
Re: [PATCH v9 4/6] init: allow initcall tables to be emitted using relative references
Hi Ard,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.18-rc2 next-20180629]
[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/Ard-Biesheuvel/arch-enable-relative-relocations-for-arm64-power-and-x86/20180627-025148
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-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=powerpc
All errors (new ones prefixed by >>):
{standard input}: Assembler messages:
>> {standard input}: Error: .size expression for .discard does not evaluate to
>> a constant
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [Update] Regression in 4.18 - 32-bit PowerPC crashes on boot - bisected to commit 1d40a5ea01d5
Le 29/06/2018 à 22:42, Larry Finger a écrit :
My PowerBook G4 Aluminum crashes on boot with 4.18-rcX kernels with a
kernel BUG at include/linux/page-flags.h:700! The problem was bisected
to commit 1d40a5ea01d5 ("mm: mark pages in use for page tables"). It is
not possible to capture the bug with anything other than a camera. The
first few lines of the traceback are as follows:
free_pgd_range+0x19c/0x30c (unreliable)
free_pgtables_0xa0/0xb0
exit_pmap+0xf4/0x16c
mmput+0x64/0xf0
do_exit+0x33c/0x89c
oops_end+0x13c/0x144
_exception_pkey+0x58/0x128
ret_from_except_full+0x0/0x4
--- interrupt: 700 at free_pgd_range+0x19c/0x30c
LR = free_pgd_range+0x19c/0x30c
free_pgtables+0xa/0xb
exit_mnap+0xf4/0x16c
mmput+0x64/0xf0
flush_old_exec+0x490/0x550
I have more information regarding this BUG. Line 700 of page-flags.h is
the macro PAGE_TYPE_OPS(Table, table). For further debugging, I manually
expanded the macro, and found that the bug line is
VM_BUG_ON_PAGE(!PageTable(page), page) in routine __ClearPageTable(),
which is called from pgtable_page_dtor() in include/linux/mm.h. I also
added a printk call to PageTable() that logs page->page_type. The
routine was called twice. The first had page_type of 0xfbff, which
would have been expected for a . The second call had 0x, which
led to the BUG.
Oh, seems to be the one I noticed and told Aneesh about
(https://patchwork.ozlabs.org/patch/922771/)
Aneesh provided the patch https://patchwork.ozlabs.org/patch/934111/ for
it, does it help ?
Christophe
Larry
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel
antivirus Avast.
https://www.avast.com/antivirus
