* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
From: Prakhar Srivastava @ 2020-06-01 4:05 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Mark Rutland, kstewart, gregkh, bhsharma, tao.li, zohar, paulus,
vincenzo.frascino, will, Rob Herring, nramas, frowand.list,
masahiroy, jmorris, takahiro.akashi, linux-arm-kernel,
catalin.marinas, serge, devicetree, pasha.tatashin, hsinyi,
tusharsu, tglx, allison, christophe.leroy, mbrugger, balajib,
dmitry.kasatkin, linux-kernel, linux-security-module, james.morse,
linux-integrity, linuxppc-dev
In-Reply-To: <87v9knpa36.fsf@morokweng.localdomain>
On 5/22/20 9:08 PM, Thiago Jung Bauermann wrote:
>
> Hello Prakhar,
>
> Prakhar Srivastava <prsriva@linux.microsoft.com> writes:
>
>> On 5/12/20 4:05 PM, Rob Herring wrote:
>>> On Wed, May 06, 2020 at 10:50:04PM -0700, Prakhar Srivastava wrote:
>>>> Hi Mark,
>>>
>>> Please don't top post.
>>>
>>>> This patch set currently only address the Pure DT implementation.
>>>> EFI and ACPI implementations will be posted in subsequent patchsets.
>>>>
>>>> The logs are intended to be carried over the kexec and once read the
>>>> logs are no longer needed and in prior conversation with James(
>>>> https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1abb@arm.com/)
>>>> the apporach of using a chosen node doesn't
>>>> support the case.
>>>>
>>>> The DT entries make the reservation permanent and thus doesnt need kernel
>>>> segments to be used for this, however using a chosen-node with
>>>> reserved memory only changes the node information but memory still is
>>>> reserved via reserved-memory section.
>>>
>>> I think Mark's point was whether it needs to be permanent. We don't
>>> hardcode the initrd address for example.
>>>
>> Thankyou for clarifying my misunderstanding, i am modelling this keeping to the
>> TPM log implementation that uses a reserved memory. I will rev up the version
>> with chosen-node support.
>> That will make the memory reservation free after use.
>
> Nice. Do you intend to use the same property that powerpc uses
> (linux,ima-kexec-buffer)?
>
I was naming it ima-buffer, but naming is not a huge concern.
I will use linux,ima-kexec-buffer.
>>>> On 5/5/20 2:59 AM, Mark Rutland wrote:
>>>>> Hi Prakhar,
>>>>>
>>>>> On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
>>>>>> IMA during kexec(kexec file load) verifies the kernel signature and measures
>>>
>>> What's IMAIMA is a LSM attempting to detect if files have been accidentally or
>> maliciously altered, both remotely and locally, it can also be used
>> to appraise a file's measurement against a "good" value stored as an extended
>> attribute, and enforce local file integrity.
>>
>> IMA also validates and measures the signers of the kernel and initrd
>> during kexec. The measurements are extended to PCR 10(configurable) and the logs
>> stored in memory, however once kexec'd the logs are lost. Kexec is used as
>> secondary boot loader in may use cases and loosing the signer
>> creates a security hole.
>>
>> This patch is an implementation to carry over the logs and making it
>> possible to remotely validate the signers of the kernel and initrd. Such a
>> support exits only in powerpc.
>> This patch makes the carry over of logs architecture independent and puts the
>> complexity in a driver.
>
> If I'm not mistaken, the code at arch/powerpc/kexec/ima.c isn't actually
> powerpc-specific. It could be moved to an arch-independent directory and
> used by any other architecture which supports device trees.
>
> I think that's the simplest way forward. And to be honest I'm still
> trying to understand why you didn't take that approach. Did you try it
> and hit some obstacle or noticed a disadvantage for your use case?
>
The approach i have in this patch set is to provide an abstraction layer
that can be called from any architecture.
However taking a deeper look only the setup dtb is probably architecture
specific, only because the architecture specific kexec sets up the
device tree. I can also move the code up in security/ima. However i do
have some concerns with layering. I am hoping you can provide me with
some guidance in this aspect, i will send you the patch i am working on
to get some early feedback.
Thanks,
Prakhar Srivastava
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
>
^ permalink raw reply
* Re: [PATCH v2] selftests: powerpc: Add test for execute-disabled pkeys
From: Sandipan Das @ 2020-06-01 2:12 UTC (permalink / raw)
To: Michael Ellerman
Cc: fweimer, aneesh.kumar, linuxram, linux-mm, linux-kselftest,
linuxppc-dev, bauerman
In-Reply-To: <1eb388dc-0fde-64f3-9c05-7f9f2a398543@linux.ibm.com>
Hi Michael,
On 01/06/20 7:29 am, Sandipan Das wrote:
> Hi Michael,
>
> Thanks for your suggestions. I had a few questions regarding some
> of them.
>
> On 29/05/20 7:18 am, Michael Ellerman wrote:
>>> [...]
>>> +
>>> +static void pkeyreg_set(unsigned long uamr)
>>> +{
>>> + asm volatile("isync; mtspr 0xd, %0; isync;" : : "r"(uamr));
>>> +}
>>
>> You can use mtspr() there, but you'll need to add the isync's yourself.
>>
>
> Would it make sense to add a new macro that adds the CSI instructions?
> Something like this.
>
> diff --git a/tools/testing/selftests/powerpc/include/reg.h b/tools/testing/selftests/powerpc/include/reg.h
> index 022c5076b2c5..d60f66380cad 100644
> --- a/tools/testing/selftests/powerpc/include/reg.h
> +++ b/tools/testing/selftests/powerpc/include/reg.h
> @@ -15,6 +15,10 @@
> #define mtspr(rn, v) asm volatile("mtspr " _str(rn) ",%0" : \
> : "r" ((unsigned long)(v)) \
> : "memory")
> +#define mtspr_csi(rn, v) ({ \
> + asm volatile("isync; mtspr " _str(rn) ",%0; isync;" : \
> + : "r" ((unsigned long)(v)) \
> + : "memory"); })
>
> #define mb() asm volatile("sync" : : : "memory");
> #define barrier() asm volatile("" : : : "memory");
>
>
> I also noticed that two of the ptrace-related pkey tests were also not
> using CSIs. I will fix those too.
>
>>> [...]
>>> + /* The following two cases will avoid SEGV_PKUERR */
>>> + ftype = -1;
>>> + fpkey = -1;
>>> +
>>> + /*
>>> + * Read an instruction word from the address when AMR bits
>>> + * are not set.
>>
>> You should explain for people who aren't familiar with the ISA that "AMR
>> bits not set" means "read/write access allowed".
>>
>>> + *
>>> + * This should not generate a fault as having PROT_EXEC
>>> + * implicitly allows reads. The pkey currently restricts
>>
>> Whether PROT_EXEC implies read is not well defined (see the man page).
>> If you want to test this case I think you'd be better off specifying
>> PROT_EXEC | PROT_READ explicitly.
>>
>
> But I guess specifying PROT_EXEC | PROT_READ defeats the purpose? I can
> tweak the passing condition though based on whether READ_IMPLIES_EXEC is
> set in the personality.
>
Sorry, I read this the other way round. This won't work.
>> [...]
>>> + FAIL_IF(faults != 0 || fcode != SEGV_ACCERR);
>>> +
>>> + /* The following three cases will generate SEGV_PKUERR */
>>> + ftype = PKEY_DISABLE_ACCESS;
>>> + fpkey = pkey;
>>> +
>>> + /*
>>> + * Read an instruction word from the address when AMR bits
>>> + * are set.
>>> + *
>>> + * This should generate a pkey fault based on AMR bits only
>>> + * as having PROT_EXEC implicitly allows reads.
>>
>> Again would be better to specify PROT_READ IMHO.
>>
>
> I can use a personality check here too.
Same here.
>
>>> + */
>>> + faults = 1;
>>> + FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
>>> + printf("read from %p, pkey is execute-disabled, access-disabled\n",
>>> + (void *) faddr);
>>> + pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
>>> + i = *faddr;
>>> + FAIL_IF(faults != 0 || fcode != SEGV_PKUERR);
>>> +
>>> + /*
>>> + * Write an instruction word to the address when AMR bits
>>> + * are set.
>>> + *
>>> + * This should generate two faults. First, a pkey fault based
>>> + * on AMR bits and then an access fault based on PROT_EXEC.
>>> + */
>>> + faults = 2;
>>
>> Setting faults to the expected value and decrementing it in the fault
>> handler is kind of weird.
>>
>> I think it would be clearer if faults was just a zero-based counter of
>> how many faults we've taken, and then you test that it's == 2 below.
>>
>>> + FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
>>> + printf("write to %p, pkey is execute-disabled, access-disabled\n",
>>> + (void *) faddr);
>>> + pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
>>> + *faddr = 0x60000000; /* nop */
>>> + FAIL_IF(faults != 0 || fcode != SEGV_ACCERR);
>>
>> ie. FAIL_IF(faults != 2 || ... )
>>
>
> Agreed, it is weird. IIRC, I did this to make sure that if the test
> kept getting repeated faults at the same address and exceeded the
> maximum number of expected faults i.e. it gets another fault when
> 'faults' is already zero, then the signal handler will attempt to
> let the program continue by giving all permissions to the page and
> also the pkey. Would it make sense to just rename 'faults' to
> something like 'remaining_faults'?
>
>
> - Sandipan
>
^ permalink raw reply
* Re: [PATCH v2] selftests: powerpc: Add test for execute-disabled pkeys
From: Sandipan Das @ 2020-06-01 1:59 UTC (permalink / raw)
To: Michael Ellerman
Cc: fweimer, aneesh.kumar, linuxram, linux-mm, linux-kselftest,
linuxppc-dev, bauerman
In-Reply-To: <87tuzzik8q.fsf@mpe.ellerman.id.au>
Hi Michael,
Thanks for your suggestions. I had a few questions regarding some
of them.
On 29/05/20 7:18 am, Michael Ellerman wrote:
>> [...]
>> +
>> +static void pkeyreg_set(unsigned long uamr)
>> +{
>> + asm volatile("isync; mtspr 0xd, %0; isync;" : : "r"(uamr));
>> +}
>
> You can use mtspr() there, but you'll need to add the isync's yourself.
>
Would it make sense to add a new macro that adds the CSI instructions?
Something like this.
diff --git a/tools/testing/selftests/powerpc/include/reg.h b/tools/testing/selftests/powerpc/include/reg.h
index 022c5076b2c5..d60f66380cad 100644
--- a/tools/testing/selftests/powerpc/include/reg.h
+++ b/tools/testing/selftests/powerpc/include/reg.h
@@ -15,6 +15,10 @@
#define mtspr(rn, v) asm volatile("mtspr " _str(rn) ",%0" : \
: "r" ((unsigned long)(v)) \
: "memory")
+#define mtspr_csi(rn, v) ({ \
+ asm volatile("isync; mtspr " _str(rn) ",%0; isync;" : \
+ : "r" ((unsigned long)(v)) \
+ : "memory"); })
#define mb() asm volatile("sync" : : : "memory");
#define barrier() asm volatile("" : : : "memory");
I also noticed that two of the ptrace-related pkey tests were also not
using CSIs. I will fix those too.
>> [...]
>> + /* The following two cases will avoid SEGV_PKUERR */
>> + ftype = -1;
>> + fpkey = -1;
>> +
>> + /*
>> + * Read an instruction word from the address when AMR bits
>> + * are not set.
>
> You should explain for people who aren't familiar with the ISA that "AMR
> bits not set" means "read/write access allowed".
>
>> + *
>> + * This should not generate a fault as having PROT_EXEC
>> + * implicitly allows reads. The pkey currently restricts
>
> Whether PROT_EXEC implies read is not well defined (see the man page).
> If you want to test this case I think you'd be better off specifying
> PROT_EXEC | PROT_READ explicitly.
>
But I guess specifying PROT_EXEC | PROT_READ defeats the purpose? I can
tweak the passing condition though based on whether READ_IMPLIES_EXEC is
set in the personality.
> [...]
>> + FAIL_IF(faults != 0 || fcode != SEGV_ACCERR);
>> +
>> + /* The following three cases will generate SEGV_PKUERR */
>> + ftype = PKEY_DISABLE_ACCESS;
>> + fpkey = pkey;
>> +
>> + /*
>> + * Read an instruction word from the address when AMR bits
>> + * are set.
>> + *
>> + * This should generate a pkey fault based on AMR bits only
>> + * as having PROT_EXEC implicitly allows reads.
>
> Again would be better to specify PROT_READ IMHO.
>
I can use a personality check here too.
>> + */
>> + faults = 1;
>> + FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
>> + printf("read from %p, pkey is execute-disabled, access-disabled\n",
>> + (void *) faddr);
>> + pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
>> + i = *faddr;
>> + FAIL_IF(faults != 0 || fcode != SEGV_PKUERR);
>> +
>> + /*
>> + * Write an instruction word to the address when AMR bits
>> + * are set.
>> + *
>> + * This should generate two faults. First, a pkey fault based
>> + * on AMR bits and then an access fault based on PROT_EXEC.
>> + */
>> + faults = 2;
>
> Setting faults to the expected value and decrementing it in the fault
> handler is kind of weird.
>
> I think it would be clearer if faults was just a zero-based counter of
> how many faults we've taken, and then you test that it's == 2 below.
>
>> + FAIL_IF(sys_pkey_mprotect(insns, pgsize, PROT_EXEC, pkey) != 0);
>> + printf("write to %p, pkey is execute-disabled, access-disabled\n",
>> + (void *) faddr);
>> + pkey_set_rights(pkey, PKEY_DISABLE_ACCESS);
>> + *faddr = 0x60000000; /* nop */
>> + FAIL_IF(faults != 0 || fcode != SEGV_ACCERR);
>
> ie. FAIL_IF(faults != 2 || ... )
>
Agreed, it is weird. IIRC, I did this to make sure that if the test
kept getting repeated faults at the same address and exceeded the
maximum number of expected faults i.e. it gets another fault when
'faults' is already zero, then the signal handler will attempt to
let the program continue by giving all permissions to the page and
also the pkey. Would it make sense to just rename 'faults' to
something like 'remaining_faults'?
- Sandipan
^ permalink raw reply related
* Re: [PATCH 8/8] macintosh/adb-iop: Implement SRQ autopolling
From: Finn Thain @ 2020-06-01 0:14 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-m68k, linuxppc-dev, Linux Kernel Mailing List,
Joshua Thompson
In-Reply-To: <CAMuHMdUjrFDob01EWC4e04tAkj5JTm_2Ei5WsPqN1eGDz=x3+Q@mail.gmail.com>
On Sun, 31 May 2020, Geert Uytterhoeven wrote:
> Hi Finn,
>
> On Sun, May 31, 2020 at 1:20 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> > The adb_driver.autopoll method is needed during ADB bus scan and device
> > address assignment. Implement this method so that the IOP's list of
> > device addresses can be updated. When the list is empty, disable SRQ
> > autopolling.
> >
> > Cc: Joshua Thompson <funaho@jurai.org>
> > Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> > Tested-by: Stan Johnson <userm57@yahoo.com>
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
>
> Thanks for your patch!
>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
Thanks for your tag.
> > arch/m68k/include/asm/adb_iop.h | 1 +
> > drivers/macintosh/adb-iop.c | 32 ++++++++++++++++++++++++++------
>
> As this header file is used by a single source file only, perhaps it
> should just be absorbed by the latter?
Sure, it could be absorbed by both asm/mac_iop.h and
drivers/macintosh/adb-iop.c but I don't see the point...
> Then you no longer need my Acked-by for future changes ;-)
>
But you shouldn't need to ack this kind of change in the first place.
IMHO, the arch/m68k/mac maintainer should be the one to ack changes to
both arch/m68k/include/asm/adb_iop.h and drivers/macintosh/adb-iop.c.
Not that I'm complaining (thanks again for your ack!)
^ permalink raw reply
* Re: [musl] Re: ppc64le and 32-bit LE userland compatibility
From: Daniel Kolesa @ 2020-05-31 22:29 UTC (permalink / raw)
To: Segher Boessenkool, musl, Will Springer
Cc: eery, libc-alpha, linuxppc-dev, Palmer Dabbelt via binutils,
libc-dev
In-Reply-To: <20200531204205.GI31009@gate.crashing.org>
On Sun, May 31, 2020, at 22:42, Segher Boessenkool wrote:
> On Sun, May 31, 2020 at 12:57:12AM +0000, Will Springer wrote:
> > On Saturday, May 30, 2020 12:22:12 PM PDT Segher Boessenkool wrote:
> > > The original sysv PowerPC supplement
> > > http://refspecs.linux-foundation.org/elf/elfspec_ppc.pdf
> > > supports LE as well, and most powerpcle ports use that. But, the
> > > big-endian Linux ABI differs in quite a few places, and it of course
> > > makes a lot better sense if powerpcle-linux follows that.
> >
> > Right, I should have clarified I was talking about Linux ABIs
> > specifically.
>
> That was the link you deleted.
>
> > > What patches did you need? I regularly build >30 cross compilers (on
> > > both BE and LE hosts; I haven't used 32-bit hosts for a long time, but
> > > in the past those worked fine as well). I also cross-built
> > > powerpcle-linux-gcc quite a few times (from powerpc64le, from powerpc64,
> > > from various x86).
> >
> > There was just an assumption that LE == powerpc64le in libgo, spotted by
> > q66 (daniel@ on the CC). I just pushed the patch to [1].
>
> Please send GCC patches to gcc-patches@ ?
FWIW, that patch alone is not very useful, we'd need to otherwise patch libgo to recognize a new GOARCH (as right now it's likely to just use 'ppc' which is wrong).
That said, we'll get back to you with any patches we have. One I can already think of - we will need to update the dynamic linker name so that it uses ld-musl-powerpcle.so instead of powerpc (musl needs to be updated the same way by adding the subarch variable for the 'le' prefix).
>
> > > Almost no project that used 32-bit PowerPC in LE mode has sent patches
> > > to the upstreams.
> >
> > Right, but I have heard concerns from at least one person familiar with
> > the ppc kernel about breaking existing users of this arch-endianness
> > combo, if any. It seems likely that none of those use upstream, though ^^;
>
> So we don't care, because we *cannot* care.
Well, that's the reason this thread was opened in the first place - to call out to any potential users, and synchronize with upstreams on a single way forward that all upstreams can agree on, since this effort requires changes in various parts of the stack. We don't want to hog changes locally or otherwise do any changes that would be in conflict with upstream projects, as that would mean needlessly diverging, which only means trouble later on.
>
> > > A huge factor in having good GCC support for powerpcle-linux (or
> > > anything else) is someone needs to regularly test it, and share test
> > > results with us (via gcc-testresults@). Hint hint hint :-)
> > >
> > > That way we know it is in good shape, know when we are regressing it,
> > > know there is interest in it.
> >
> > Once I have more of a bootstrapped userland than a barely-functional
> > cross chroot, I'll get back to you on that :)
>
> Cool! Looking forward to it.
>
> Thanks,
Either way, thanks for the hints so far.
>
>
> Segher
>
Daniel (q66)
^ permalink raw reply
* Re: [PATCH 8/8] macintosh/adb-iop: Implement SRQ autopolling
From: Brad Boyer @ 2020-05-31 20:01 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: linux-m68k, linuxppc-dev, Joshua Thompson, Finn Thain
In-Reply-To: <CAMuHMdUjrFDob01EWC4e04tAkj5JTm_2Ei5WsPqN1eGDz=x3+Q@mail.gmail.com>
On Sun, May 31, 2020 at 10:38:04AM +0200, Geert Uytterhoeven wrote:
> > arch/m68k/include/asm/adb_iop.h | 1 +
>
> As this header file is used by a single source file only, perhaps it should
> just be absorbed by the latter?
> Then you no longer need my Acked-by for future changes ;-)
While I don't really feel involved in this specific change (although I
was one of the testers when the driver was first written), I am a little
curious about the current coding standards. This header is pretty much
just a declaration of the hardware interface, of which there are many
in this directory. Is it better to just define all the offsets and bits
for hardware registers inside the driver? We used to always put them in
headers like this, but is that not the current standard? Would it be
cleaner to put such headers in the directory with the driver effectively
making them private? I seem to see quite a bit of that as well.
Thank you for your advice.
Brad Boyer
flar@allandria.com
^ permalink raw reply
* Re: ppc64le and 32-bit LE userland compatibility
From: Segher Boessenkool @ 2020-05-31 20:42 UTC (permalink / raw)
To: Will Springer
Cc: libc-alpha, eery, daniel, musl, binutils, libc-dev, linuxppc-dev
In-Reply-To: <2956705.fEcJ0Lxnt5@sheen>
On Sun, May 31, 2020 at 12:57:12AM +0000, Will Springer wrote:
> On Saturday, May 30, 2020 12:22:12 PM PDT Segher Boessenkool wrote:
> > The original sysv PowerPC supplement
> > http://refspecs.linux-foundation.org/elf/elfspec_ppc.pdf
> > supports LE as well, and most powerpcle ports use that. But, the
> > big-endian Linux ABI differs in quite a few places, and it of course
> > makes a lot better sense if powerpcle-linux follows that.
>
> Right, I should have clarified I was talking about Linux ABIs
> specifically.
That was the link you deleted.
> > What patches did you need? I regularly build >30 cross compilers (on
> > both BE and LE hosts; I haven't used 32-bit hosts for a long time, but
> > in the past those worked fine as well). I also cross-built
> > powerpcle-linux-gcc quite a few times (from powerpc64le, from powerpc64,
> > from various x86).
>
> There was just an assumption that LE == powerpc64le in libgo, spotted by
> q66 (daniel@ on the CC). I just pushed the patch to [1].
Please send GCC patches to gcc-patches@ ?
> > Almost no project that used 32-bit PowerPC in LE mode has sent patches
> > to the upstreams.
>
> Right, but I have heard concerns from at least one person familiar with
> the ppc kernel about breaking existing users of this arch-endianness
> combo, if any. It seems likely that none of those use upstream, though ^^;
So we don't care, because we *cannot* care.
> > A huge factor in having good GCC support for powerpcle-linux (or
> > anything else) is someone needs to regularly test it, and share test
> > results with us (via gcc-testresults@). Hint hint hint :-)
> >
> > That way we know it is in good shape, know when we are regressing it,
> > know there is interest in it.
>
> Once I have more of a bootstrapped userland than a barely-functional
> cross chroot, I'll get back to you on that :)
Cool! Looking forward to it.
Thanks,
Segher
^ permalink raw reply
* Re: [PATCH v4 5/8] powerpc/pmem/of_pmem: Update of_pmem to use the new barrier instruction.
From: kbuild test robot @ 2020-05-31 14:46 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe, linux-nvdimm
Cc: Aneesh Kumar K.V, dan.j.williams, kbuild-all, oohall
In-Reply-To: <20200529052820.151651-6-aneesh.kumar@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 4053 bytes --]
Hi "Aneesh,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on linux-nvdimm/libnvdimm-for-next scottwood/next v5.7-rc7]
[cannot apply to next-20200529]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Aneesh-Kumar-K-V/Support-new-pmem-flush-and-sync-instructions-for-POWER/20200531-062841
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-ppc6xx_defconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
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
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
In file included from arch/powerpc/include/asm/asm-prototypes.h:12,
from arch/powerpc/kernel/early_32.c:11:
arch/powerpc/include/asm/cacheflush.h: In function 'arch_pmem_flush_barrier':
arch/powerpc/include/asm/cacheflush.h:126:6: error: implicit declaration of function 'cpu_has_feature'; did you mean 'mmu_has_feature'? [-Werror=implicit-function-declaration]
126 | if (cpu_has_feature(CPU_FTR_ARCH_207S))
| ^~~~~~~~~~~~~~~
| mmu_has_feature
In file included from arch/powerpc/include/asm/cputhreads.h:7,
from arch/powerpc/include/asm/mmu_context.h:12,
from arch/powerpc/include/asm/asm-prototypes.h:17,
from arch/powerpc/kernel/early_32.c:11:
arch/powerpc/include/asm/cpu_has_feature.h: At top level:
>> arch/powerpc/include/asm/cpu_has_feature.h:49:20: error: conflicting types for 'cpu_has_feature'
49 | static inline bool cpu_has_feature(unsigned long feature)
| ^~~~~~~~~~~~~~~
In file included from arch/powerpc/include/asm/asm-prototypes.h:12,
from arch/powerpc/kernel/early_32.c:11:
arch/powerpc/include/asm/cacheflush.h:126:6: note: previous implicit declaration of 'cpu_has_feature' was here
126 | if (cpu_has_feature(CPU_FTR_ARCH_207S))
| ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/cpu_has_feature +49 arch/powerpc/include/asm/cpu_has_feature.h
c812c7d8f1470a Aneesh Kumar K.V 2016-07-23 38
4db7327194dba0 Kevin Hao 2016-07-23 39 if (CPU_FTRS_ALWAYS & feature)
4db7327194dba0 Kevin Hao 2016-07-23 40 return true;
4db7327194dba0 Kevin Hao 2016-07-23 41
4db7327194dba0 Kevin Hao 2016-07-23 42 if (!(CPU_FTRS_POSSIBLE & feature))
4db7327194dba0 Kevin Hao 2016-07-23 43 return false;
4db7327194dba0 Kevin Hao 2016-07-23 44
4db7327194dba0 Kevin Hao 2016-07-23 45 i = __builtin_ctzl(feature);
4db7327194dba0 Kevin Hao 2016-07-23 46 return static_branch_likely(&cpu_feature_keys[i]);
4db7327194dba0 Kevin Hao 2016-07-23 47 }
4db7327194dba0 Kevin Hao 2016-07-23 48 #else
b92a226e528423 Kevin Hao 2016-07-23 @49 static inline bool cpu_has_feature(unsigned long feature)
b92a226e528423 Kevin Hao 2016-07-23 50 {
b92a226e528423 Kevin Hao 2016-07-23 51 return early_cpu_has_feature(feature);
b92a226e528423 Kevin Hao 2016-07-23 52 }
4db7327194dba0 Kevin Hao 2016-07-23 53 #endif
b92a226e528423 Kevin Hao 2016-07-23 54
:::::: The code at line 49 was first introduced by commit
:::::: b92a226e528423b8d249dd09bb450d53361fbfcb powerpc: Move cpu_has_feature() to a separate file
:::::: TO: Kevin Hao <haokexin@gmail.com>
:::::: CC: Michael Ellerman <mpe@ellerman.id.au>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31557 bytes --]
^ permalink raw reply
* Re: [PATCH 8/8] macintosh/adb-iop: Implement SRQ autopolling
From: Geert Uytterhoeven @ 2020-05-31 8:38 UTC (permalink / raw)
To: Finn Thain
Cc: linux-m68k, linuxppc-dev, Linux Kernel Mailing List,
Joshua Thompson
In-Reply-To: <0fb7fdcd99d7820bb27faf1f27f7f6f1923914ef.1590880623.git.fthain@telegraphics.com.au>
Hi Finn,
On Sun, May 31, 2020 at 1:20 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> The adb_driver.autopoll method is needed during ADB bus scan and device
> address assignment. Implement this method so that the IOP's list of
> device addresses can be updated. When the list is empty, disable SRQ
> autopolling.
>
> Cc: Joshua Thompson <funaho@jurai.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Thanks for your patch!
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> arch/m68k/include/asm/adb_iop.h | 1 +
> drivers/macintosh/adb-iop.c | 32 ++++++++++++++++++++++++++------
As this header file is used by a single source file only, perhaps it should
just be absorbed by the latter?
Then you no longer need my Acked-by for future changes ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH 5/8] macintosh/adb-iop: Resolve static checker warnings
From: Finn Thain @ 2020-05-30 23:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-m68k, linuxppc-dev, linux-kernel, Joshua Thompson
In-Reply-To: <cover.1590880623.git.fthain@telegraphics.com.au>
drivers/macintosh/adb-iop.c:215:28: warning: Using plain integer as NULL pointer
drivers/macintosh/adb-iop.c:170:5: warning: symbol 'adb_iop_probe' was not declared. Should it be static?
drivers/macintosh/adb-iop.c:177:5: warning: symbol 'adb_iop_init' was not declared. Should it be static?
drivers/macintosh/adb-iop.c:184:5: warning: symbol 'adb_iop_send_request' was not declared. Should it be static?
drivers/macintosh/adb-iop.c:230:5: warning: symbol 'adb_iop_autopoll' was not declared. Should it be static?
drivers/macintosh/adb-iop.c:236:6: warning: symbol 'adb_iop_poll' was not declared. Should it be static?
drivers/macintosh/adb-iop.c:241:5: warning: symbol 'adb_iop_reset_bus' was not declared. Should it be static?
Cc: Joshua Thompson <funaho@jurai.org>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
drivers/macintosh/adb-iop.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index 7ecc41bc7358..a2b28e09f2ce 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -166,21 +166,21 @@ static void adb_iop_start(void)
adb_iop_complete);
}
-int adb_iop_probe(void)
+static int adb_iop_probe(void)
{
if (!iop_ism_present)
return -ENODEV;
return 0;
}
-int adb_iop_init(void)
+static int adb_iop_init(void)
{
pr_info("adb: IOP ISM driver v0.4 for Unified ADB\n");
iop_listen(ADB_IOP, ADB_CHAN, adb_iop_listen, "ADB");
return 0;
}
-int adb_iop_send_request(struct adb_request *req, int sync)
+static int adb_iop_send_request(struct adb_request *req, int sync)
{
int err;
@@ -211,7 +211,7 @@ static int adb_iop_write(struct adb_request *req)
local_irq_save(flags);
- if (current_req != 0) {
+ if (current_req) {
last_req->next = req;
last_req = req;
} else {
@@ -227,18 +227,18 @@ static int adb_iop_write(struct adb_request *req)
return 0;
}
-int adb_iop_autopoll(int devs)
+static int adb_iop_autopoll(int devs)
{
/* TODO: how do we enable/disable autopoll? */
return 0;
}
-void adb_iop_poll(void)
+static void adb_iop_poll(void)
{
iop_ism_irq_poll(ADB_IOP);
}
-int adb_iop_reset_bus(void)
+static int adb_iop_reset_bus(void)
{
struct adb_request req;
--
2.26.2
^ permalink raw reply related
* [PATCH 7/8] macintosh/adb-iop: Implement sending -> idle state transition
From: Finn Thain @ 2020-05-30 23:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-m68k, linuxppc-dev, linux-kernel, Joshua Thompson
In-Reply-To: <cover.1590880623.git.fthain@telegraphics.com.au>
On leaving the 'sending' state, proceed to the 'idle' state if no reply is
expected. Drop redundant test for adb_iop_state == sending && current_req.
Cc: Joshua Thompson <funaho@jurai.org>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
drivers/macintosh/adb-iop.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index f22792c95d4f..8594e4f9a830 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -77,15 +77,14 @@ static void adb_iop_done(void)
static void adb_iop_complete(struct iop_msg *msg)
{
- struct adb_request *req;
unsigned long flags;
local_irq_save(flags);
- req = current_req;
- if ((adb_iop_state == sending) && req && req->reply_expected) {
+ if (current_req->reply_expected)
adb_iop_state = awaiting_reply;
- }
+ else
+ adb_iop_done();
local_irq_restore(flags);
}
--
2.26.2
^ permalink raw reply related
* [PATCH 4/8] macintosh/adb-iop: Access current_req and adb_iop_state when inside lock
From: Finn Thain @ 2020-05-30 23:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-m68k, linuxppc-dev, linux-kernel, Joshua Thompson
In-Reply-To: <cover.1590880623.git.fthain@telegraphics.com.au>
Drop the redundant local_irq_save/restore() from adb_iop_start() because
the caller has to do it anyway. This is the pattern used in via-macii.
Cc: Joshua Thompson <funaho@jurai.org>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
drivers/macintosh/adb-iop.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index c3089dacf2e2..7ecc41bc7358 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -137,7 +137,6 @@ static void adb_iop_listen(struct iop_msg *msg)
static void adb_iop_start(void)
{
- unsigned long flags;
struct adb_request *req;
struct adb_iopmsg amsg;
@@ -146,8 +145,6 @@ static void adb_iop_start(void)
if (!req)
return;
- local_irq_save(flags);
-
/* The IOP takes MacII-style packets, so strip the initial
* ADB_PACKET byte.
*/
@@ -161,7 +158,6 @@ static void adb_iop_start(void)
req->sent = 1;
adb_iop_state = sending;
- local_irq_restore(flags);
/* Now send it. The IOP manager will call adb_iop_complete
* when the message has been sent.
@@ -208,13 +204,13 @@ static int adb_iop_write(struct adb_request *req)
return -EINVAL;
}
- local_irq_save(flags);
-
req->next = NULL;
req->sent = 0;
req->complete = 0;
req->reply_len = 0;
+ local_irq_save(flags);
+
if (current_req != 0) {
last_req->next = req;
last_req = req;
@@ -223,10 +219,11 @@ static int adb_iop_write(struct adb_request *req)
last_req = req;
}
- local_irq_restore(flags);
-
if (adb_iop_state == idle)
adb_iop_start();
+
+ local_irq_restore(flags);
+
return 0;
}
--
2.26.2
^ permalink raw reply related
* [PATCH 3/8] macintosh/adb-iop: Adopt bus reset algorithm from via-macii driver
From: Finn Thain @ 2020-05-30 23:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-m68k, linuxppc-dev, linux-kernel, Joshua Thompson
In-Reply-To: <cover.1590880623.git.fthain@telegraphics.com.au>
This algorithm is slightly shorter and avoids the surprising
adb_iop_start() call in adb_iop_poll().
Cc: Joshua Thompson <funaho@jurai.org>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
drivers/macintosh/adb-iop.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index ca3b411b0742..c3089dacf2e2 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -238,24 +238,19 @@ int adb_iop_autopoll(int devs)
void adb_iop_poll(void)
{
- if (adb_iop_state == idle)
- adb_iop_start();
iop_ism_irq_poll(ADB_IOP);
}
int adb_iop_reset_bus(void)
{
- struct adb_request req = {
- .reply_expected = 0,
- .nbytes = 2,
- .data = { ADB_PACKET, 0 },
- };
-
- adb_iop_write(&req);
- while (!req.complete) {
- adb_iop_poll();
- schedule();
- }
+ struct adb_request req;
+
+ /* Command = 0, Address = ignored */
+ adb_request(&req, NULL, ADBREQ_NOSEND, 1, ADB_BUSRESET);
+ adb_iop_send_request(&req, 1);
+
+ /* Don't want any more requests during the Global Reset low time. */
+ mdelay(3);
return 0;
}
--
2.26.2
^ permalink raw reply related
* [PATCH 2/8] macintosh/adb-iop: Correct comment text
From: Finn Thain @ 2020-05-30 23:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-m68k, linuxppc-dev, linux-kernel, Joshua Thompson
In-Reply-To: <cover.1590880623.git.fthain@telegraphics.com.au>
This patch improves comment style and corrects some misunderstandings
in the text.
Cc: Joshua Thompson <funaho@jurai.org>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
drivers/macintosh/adb-iop.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index ce28ff40fb9c..ca3b411b0742 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -101,11 +101,10 @@ static void adb_iop_listen(struct iop_msg *msg)
req = current_req;
- /* Handle a timeout. Timeout packets seem to occur even after */
- /* we've gotten a valid reply to a TALK, so I'm assuming that */
- /* a "timeout" is actually more like an "end-of-data" signal. */
- /* We need to send back a timeout packet to the IOP to shut */
- /* it up, plus complete the current request, if any. */
+ /* Handle a timeout. Timeout packets seem to occur even after
+ * we've gotten a valid reply to a TALK, presumably because of
+ * autopolling.
+ */
if (amsg->flags & ADB_IOP_TIMEOUT) {
msg->reply[0] = ADB_IOP_TIMEOUT | ADB_IOP_AUTOPOLL;
@@ -115,9 +114,6 @@ static void adb_iop_listen(struct iop_msg *msg)
adb_iop_end_req(req, idle);
}
} else {
- /* TODO: is it possible for more than one chunk of data */
- /* to arrive before the timeout? If so we need to */
- /* use reply_ptr here like the other drivers do. */
if ((adb_iop_state == awaiting_reply) &&
(amsg->flags & ADB_IOP_EXPLICIT)) {
req->reply_len = amsg->count + 1;
@@ -152,23 +148,24 @@ static void adb_iop_start(void)
local_irq_save(flags);
- /* The IOP takes MacII-style packets, so */
- /* strip the initial ADB_PACKET byte. */
-
+ /* The IOP takes MacII-style packets, so strip the initial
+ * ADB_PACKET byte.
+ */
amsg.flags = ADB_IOP_EXPLICIT;
amsg.count = req->nbytes - 2;
- /* amsg.data immediately follows amsg.cmd, effectively making */
- /* amsg.cmd a pointer to the beginning of a full ADB packet. */
+ /* amsg.data immediately follows amsg.cmd, effectively making
+ * &amsg.cmd a pointer to the beginning of a full ADB packet.
+ */
memcpy(&amsg.cmd, req->data + 1, req->nbytes - 1);
req->sent = 1;
adb_iop_state = sending;
local_irq_restore(flags);
- /* Now send it. The IOP manager will call adb_iop_complete */
- /* when the packet has been sent. */
-
+ /* Now send it. The IOP manager will call adb_iop_complete
+ * when the message has been sent.
+ */
iop_send_message(ADB_IOP, ADB_CHAN, req, sizeof(amsg), (__u8 *)&amsg,
adb_iop_complete);
}
--
2.26.2
^ permalink raw reply related
* [PATCH 1/8] macintosh/adb-iop: Remove dead and redundant code
From: Finn Thain @ 2020-05-30 23:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-m68k, linuxppc-dev, linux-kernel, Joshua Thompson
In-Reply-To: <cover.1590880623.git.fthain@telegraphics.com.au>
Cc: Joshua Thompson <funaho@jurai.org>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
drivers/macintosh/adb-iop.c | 29 -----------------------------
1 file changed, 29 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index fca31640e3ef..ce28ff40fb9c 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -18,24 +18,16 @@
#include <linux/mm.h>
#include <linux/delay.h>
#include <linux/init.h>
-#include <linux/proc_fs.h>
#include <asm/macintosh.h>
#include <asm/macints.h>
#include <asm/mac_iop.h>
-#include <asm/mac_oss.h>
#include <asm/adb_iop.h>
#include <linux/adb.h>
-/*#define DEBUG_ADB_IOP*/
-
static struct adb_request *current_req;
static struct adb_request *last_req;
-#if 0
-static unsigned char reply_buff[16];
-static unsigned char *reply_ptr;
-#endif
static enum adb_iop_state {
idle,
@@ -104,22 +96,11 @@ static void adb_iop_listen(struct iop_msg *msg)
struct adb_iopmsg *amsg = (struct adb_iopmsg *)msg->message;
struct adb_request *req;
unsigned long flags;
-#ifdef DEBUG_ADB_IOP
- int i;
-#endif
local_irq_save(flags);
req = current_req;
-#ifdef DEBUG_ADB_IOP
- printk("adb_iop_listen %p: rcvd packet, %d bytes: %02X %02X", req,
- (uint)amsg->count + 2, (uint)amsg->flags, (uint)amsg->cmd);
- for (i = 0; i < amsg->count; i++)
- printk(" %02X", (uint)amsg->data[i]);
- printk("\n");
-#endif
-
/* Handle a timeout. Timeout packets seem to occur even after */
/* we've gotten a valid reply to a TALK, so I'm assuming that */
/* a "timeout" is actually more like an "end-of-data" signal. */
@@ -163,9 +144,6 @@ static void adb_iop_start(void)
unsigned long flags;
struct adb_request *req;
struct adb_iopmsg amsg;
-#ifdef DEBUG_ADB_IOP
- int i;
-#endif
/* get the packet to send */
req = current_req;
@@ -174,13 +152,6 @@ static void adb_iop_start(void)
local_irq_save(flags);
-#ifdef DEBUG_ADB_IOP
- printk("adb_iop_start %p: sending packet, %d bytes:", req, req->nbytes);
- for (i = 0; i < req->nbytes; i++)
- printk(" %02X", (uint)req->data[i]);
- printk("\n");
-#endif
-
/* The IOP takes MacII-style packets, so */
/* strip the initial ADB_PACKET byte. */
--
2.26.2
^ permalink raw reply related
* [PATCH 0/8] Mac ADB IOP driver fixes
From: Finn Thain @ 2020-05-30 23:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Geert Uytterhoeven, linux-m68k, linuxppc-dev, linux-kernel,
Joshua Thompson
The adb-iop driver was never finished. Some deficiencies have become
apparent over the years. For example,
- Mouse and/or keyboard may stop working if used together
- SRQ autopoll list cannot be changed
- Some bugs were found by inspection
This patch series contains fixes for the known bugs in the driver, plus
a few clean-ups.
Finn Thain (8):
macintosh/adb-iop: Remove dead and redundant code
macintosh/adb-iop: Correct comment text
macintosh/adb-iop: Adopt bus reset algorithm from via-macii driver
macintosh/adb-iop: Access current_req and adb_iop_state when inside
lock
macintosh/adb-iop: Resolve static checker warnings
macintosh/adb-iop: Implement idle -> sending state transition
macintosh/adb-iop: Implement sending -> idle state transition
macintosh/adb-iop: Implement SRQ autopolling
arch/m68k/include/asm/adb_iop.h | 1 +
drivers/macintosh/adb-iop.c | 186 +++++++++++++++-----------------
2 files changed, 86 insertions(+), 101 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH 6/8] macintosh/adb-iop: Implement idle -> sending state transition
From: Finn Thain @ 2020-05-30 23:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-m68k, linuxppc-dev, linux-kernel, Joshua Thompson
In-Reply-To: <cover.1590880623.git.fthain@telegraphics.com.au>
In the present algorithm, the 'idle' state transition does not take
place until there's a bus timeout. Once idle, the driver does not
automatically proceed with the next request.
Change the algorithm so that queued ADB requests will be sent as soon as
the driver becomes idle. This is to take place after the current IOP
message is completed.
Cc: Joshua Thompson <funaho@jurai.org>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
drivers/macintosh/adb-iop.c | 43 +++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index a2b28e09f2ce..f22792c95d4f 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -54,13 +54,19 @@ struct adb_driver adb_iop_driver = {
.reset_bus = adb_iop_reset_bus
};
-static void adb_iop_end_req(struct adb_request *req, int state)
+static void adb_iop_done(void)
{
+ struct adb_request *req = current_req;
+
+ adb_iop_state = idle;
+
req->complete = 1;
current_req = req->next;
if (req->done)
(*req->done)(req);
- adb_iop_state = state;
+
+ if (adb_iop_state == idle)
+ adb_iop_start();
}
/*
@@ -94,37 +100,36 @@ static void adb_iop_complete(struct iop_msg *msg)
static void adb_iop_listen(struct iop_msg *msg)
{
struct adb_iopmsg *amsg = (struct adb_iopmsg *)msg->message;
- struct adb_request *req;
unsigned long flags;
+ bool req_done = false;
local_irq_save(flags);
- req = current_req;
-
/* Handle a timeout. Timeout packets seem to occur even after
* we've gotten a valid reply to a TALK, presumably because of
* autopolling.
*/
- if (amsg->flags & ADB_IOP_TIMEOUT) {
- msg->reply[0] = ADB_IOP_TIMEOUT | ADB_IOP_AUTOPOLL;
- msg->reply[1] = 0;
- msg->reply[2] = 0;
- if (req && (adb_iop_state != idle)) {
- adb_iop_end_req(req, idle);
- }
- } else {
- if ((adb_iop_state == awaiting_reply) &&
- (amsg->flags & ADB_IOP_EXPLICIT)) {
+ if (amsg->flags & ADB_IOP_EXPLICIT) {
+ if (adb_iop_state == awaiting_reply) {
+ struct adb_request *req = current_req;
+
req->reply_len = amsg->count + 1;
memcpy(req->reply, &amsg->cmd, req->reply_len);
- } else {
- adb_input(&amsg->cmd, amsg->count + 1,
- amsg->flags & ADB_IOP_AUTOPOLL);
+
+ req_done = true;
}
- memcpy(msg->reply, msg->message, IOP_MSG_LEN);
+ } else if (!(amsg->flags & ADB_IOP_TIMEOUT)) {
+ adb_input(&amsg->cmd, amsg->count + 1,
+ amsg->flags & ADB_IOP_AUTOPOLL);
}
+
+ msg->reply[0] = ADB_IOP_AUTOPOLL;
iop_complete_message(msg);
+
+ if (req_done)
+ adb_iop_done();
+
local_irq_restore(flags);
}
--
2.26.2
^ permalink raw reply related
* [PATCH 8/8] macintosh/adb-iop: Implement SRQ autopolling
From: Finn Thain @ 2020-05-30 23:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, linux-m68k, Geert Uytterhoeven, linux-kernel,
Joshua Thompson
In-Reply-To: <cover.1590880623.git.fthain@telegraphics.com.au>
The adb_driver.autopoll method is needed during ADB bus scan and device
address assignment. Implement this method so that the IOP's list of
device addresses can be updated. When the list is empty, disable SRQ
autopolling.
Cc: Joshua Thompson <funaho@jurai.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
arch/m68k/include/asm/adb_iop.h | 1 +
drivers/macintosh/adb-iop.c | 32 ++++++++++++++++++++++++++------
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/arch/m68k/include/asm/adb_iop.h b/arch/m68k/include/asm/adb_iop.h
index 195d7fb1268c..6aecd020e2fc 100644
--- a/arch/m68k/include/asm/adb_iop.h
+++ b/arch/m68k/include/asm/adb_iop.h
@@ -29,6 +29,7 @@
#define ADB_IOP_EXPLICIT 0x80 /* nonzero if explicit command */
#define ADB_IOP_AUTOPOLL 0x40 /* auto/SRQ polling enabled */
+#define ADB_IOP_SET_AUTOPOLL 0x20 /* set autopoll device list */
#define ADB_IOP_SRQ 0x04 /* SRQ detected */
#define ADB_IOP_TIMEOUT 0x02 /* nonzero if timeout */
diff --git a/drivers/macintosh/adb-iop.c b/drivers/macintosh/adb-iop.c
index 8594e4f9a830..f3d1a460fbce 100644
--- a/drivers/macintosh/adb-iop.c
+++ b/drivers/macintosh/adb-iop.c
@@ -7,10 +7,6 @@
* 1999-07-01 (jmt) - First implementation for new driver architecture.
*
* 1999-07-31 (jmt) - First working version.
- *
- * TODO:
- *
- * o Implement SRQ handling.
*/
#include <linux/types.h>
@@ -28,6 +24,7 @@
static struct adb_request *current_req;
static struct adb_request *last_req;
+static unsigned int autopoll_devs;
static enum adb_iop_state {
idle,
@@ -123,7 +120,7 @@ static void adb_iop_listen(struct iop_msg *msg)
amsg->flags & ADB_IOP_AUTOPOLL);
}
- msg->reply[0] = ADB_IOP_AUTOPOLL;
+ msg->reply[0] = autopoll_devs ? ADB_IOP_AUTOPOLL : 0;
iop_complete_message(msg);
if (req_done)
@@ -231,9 +228,32 @@ static int adb_iop_write(struct adb_request *req)
return 0;
}
+static void adb_iop_set_ap_complete(struct iop_msg *msg)
+{
+ struct adb_iopmsg *amsg = (struct adb_iopmsg *)msg->message;
+
+ autopoll_devs = (amsg->data[1] << 8) | amsg->data[0];
+}
+
static int adb_iop_autopoll(int devs)
{
- /* TODO: how do we enable/disable autopoll? */
+ struct adb_iopmsg amsg;
+ unsigned long flags;
+ unsigned int mask = (unsigned int)devs & 0xFFFE;
+
+ local_irq_save(flags);
+
+ amsg.flags = ADB_IOP_SET_AUTOPOLL | (mask ? ADB_IOP_AUTOPOLL : 0);
+ amsg.count = 2;
+ amsg.cmd = 0;
+ amsg.data[0] = mask & 0xFF;
+ amsg.data[1] = (mask >> 8) & 0xFF;
+
+ iop_send_message(ADB_IOP, ADB_CHAN, NULL, sizeof(amsg), (__u8 *)&amsg,
+ adb_iop_set_ap_complete);
+
+ local_irq_restore(flags);
+
return 0;
}
--
2.26.2
^ permalink raw reply related
* [PATCH v1 4/4] KVM: PPC: Book3S HV: migrate hot plugged memory
From: Ram Pai @ 2020-05-31 2:27 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev
Cc: ldufour, linuxram, cclaudio, bharata, aneesh.kumar, sukadev,
bauerman, david
In-Reply-To: <1590892071-25549-1-git-send-email-linuxram@us.ibm.com>
From: Laurent Dufour <ldufour@linux.ibm.com>
When a memory slot is hot plugged to a SVM, GFNs associated with that
memory slot automatically default to secure GFN. Hence migrate the
PFNs associated with these GFNs to device-PFNs.
uv_migrate_mem_slot() is called to achieve that. It will not call
UV_PAGE_IN since this request is ignored by the Ultravisor.
NOTE: Ultravisor does not trust any page content provided by
the Hypervisor, ones the VM turns secure.
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bharata B Rao <bharata@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Claudio Carvalho <cclaudio@linux.ibm.com>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
(fixed merge conflicts. Modified the commit message)
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 4 ++++
arch/powerpc/kvm/book3s_hv.c | 11 +++++++----
arch/powerpc/kvm/book3s_hv_uvmem.c | 3 +--
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_uvmem.h b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
index f0c5708..2ec2e5afb 100644
--- a/arch/powerpc/include/asm/kvm_book3s_uvmem.h
+++ b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
@@ -23,6 +23,7 @@ unsigned long kvmppc_h_svm_page_out(struct kvm *kvm,
void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
struct kvm *kvm, bool skip_page_out,
bool purge_gfn);
+int uv_migrate_mem_slot(struct kvm *kvm, const struct kvm_memory_slot *memslot);
#else
static inline int kvmppc_uvmem_init(void)
{
@@ -78,5 +79,8 @@ static inline int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gfn)
kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
struct kvm *kvm, bool skip_page_out,
bool purge_gfn) { }
+
+static int uv_migrate_mem_slot(struct kvm *kvm,
+ const struct kvm_memory_slot *memslot);
#endif /* CONFIG_PPC_UV */
#endif /* __ASM_KVM_BOOK3S_UVMEM_H__ */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 4c62bfe..604d062 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4516,13 +4516,16 @@ static void kvmppc_core_commit_memory_region_hv(struct kvm *kvm,
case KVM_MR_CREATE:
if (kvmppc_uvmem_slot_init(kvm, new))
return;
- uv_register_mem_slot(kvm->arch.lpid,
- new->base_gfn << PAGE_SHIFT,
- new->npages * PAGE_SIZE,
- 0, new->id);
+ if (uv_register_mem_slot(kvm->arch.lpid,
+ new->base_gfn << PAGE_SHIFT,
+ new->npages * PAGE_SIZE,
+ 0, new->id))
+ return;
+ uv_migrate_mem_slot(kvm, new);
break;
case KVM_MR_DELETE:
uv_unregister_mem_slot(kvm->arch.lpid, old->id);
+ kvmppc_uvmem_drop_pages(old, kvm, true, true);
kvmppc_uvmem_slot_free(kvm, old);
break;
default:
diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
index 36dda1d..1fa5f2a 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -377,8 +377,7 @@ static int kvmppc_svm_migrate_page(struct vm_area_struct *vma,
return ret;
}
-static int uv_migrate_mem_slot(struct kvm *kvm,
- const struct kvm_memory_slot *memslot)
+int uv_migrate_mem_slot(struct kvm *kvm, const struct kvm_memory_slot *memslot)
{
unsigned long gfn = memslot->base_gfn;
unsigned long end;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v1 3/4] KVM: PPC: Book3S HV: migrate remaining normal-GFNs to secure-GFNs in H_SVM_INIT_DONE
From: Ram Pai @ 2020-05-31 2:27 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev
Cc: ldufour, linuxram, cclaudio, bharata, aneesh.kumar, sukadev,
bauerman, david
In-Reply-To: <1590892071-25549-1-git-send-email-linuxram@us.ibm.com>
H_SVM_INIT_DONE incorrectly assumes that the Ultravisor has explicitly
called H_SVM_PAGE_IN for all secure pages. These GFNs continue to be
normal GFNs associated with normal PFNs; when infact, these GFNs should
have been secure GFNs associated with device PFNs.
Move all the PFN associated with the SVM's GFNs to device PFNs, in
H_SVM_INIT_DONE. Skip the GFNs that are already Paged-in or Shared
through H_SVM_PAGE_IN.
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bharata B Rao <bharata@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Claudio Carvalho <cclaudio@linux.ibm.com>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
Documentation/powerpc/ultravisor.rst | 2 +
arch/powerpc/kvm/book3s_hv_uvmem.c | 219 ++++++++++++++++++++++++-----------
2 files changed, 154 insertions(+), 67 deletions(-)
diff --git a/Documentation/powerpc/ultravisor.rst b/Documentation/powerpc/ultravisor.rst
index 363736d..3bc8957 100644
--- a/Documentation/powerpc/ultravisor.rst
+++ b/Documentation/powerpc/ultravisor.rst
@@ -933,6 +933,8 @@ Return values
* H_UNSUPPORTED if called from the wrong context (e.g.
from an SVM or before an H_SVM_INIT_START
hypercall).
+ * H_STATE if the hypervisor could not successfully
+ transition the VM to Secure VM.
Description
~~~~~~~~~~~
diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
index 2ef1e03..36dda1d 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -318,14 +318,149 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
return ret;
}
+static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm);
+
+/*
+ * Alloc a PFN from private device memory pool. If @pagein is true,
+ * copy page from normal memory to secure memory using UV_PAGE_IN uvcall.
+ */
+static int kvmppc_svm_migrate_page(struct vm_area_struct *vma,
+ unsigned long start,
+ unsigned long end, unsigned long gpa, struct kvm *kvm,
+ unsigned long page_shift,
+ bool pagein)
+{
+ unsigned long src_pfn, dst_pfn = 0;
+ struct migrate_vma mig;
+ struct page *dpage;
+ struct page *spage;
+ unsigned long pfn;
+ int ret = 0;
+
+ memset(&mig, 0, sizeof(mig));
+ mig.vma = vma;
+ mig.start = start;
+ mig.end = end;
+ mig.src = &src_pfn;
+ mig.dst = &dst_pfn;
+
+ ret = migrate_vma_setup(&mig);
+ if (ret)
+ return ret;
+
+ if (!(*mig.src & MIGRATE_PFN_MIGRATE)) {
+ ret = -1;
+ goto out_finalize;
+ }
+
+ dpage = kvmppc_uvmem_get_page(gpa, kvm);
+ if (!dpage) {
+ ret = -1;
+ goto out_finalize;
+ }
+
+ if (pagein) {
+ pfn = *mig.src >> MIGRATE_PFN_SHIFT;
+ spage = migrate_pfn_to_page(*mig.src);
+ if (spage) {
+ ret = uv_page_in(kvm->arch.lpid, pfn << page_shift,
+ gpa, 0, page_shift);
+ if (ret)
+ goto out_finalize;
+ }
+ }
+
+ *mig.dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
+ migrate_vma_pages(&mig);
+out_finalize:
+ migrate_vma_finalize(&mig);
+ return ret;
+}
+
+static int uv_migrate_mem_slot(struct kvm *kvm,
+ const struct kvm_memory_slot *memslot)
+{
+ unsigned long gfn = memslot->base_gfn;
+ unsigned long end;
+ bool downgrade = false;
+ struct vm_area_struct *vma;
+ int i, ret = 0;
+ unsigned long start = gfn_to_hva(kvm, gfn);
+
+ if (kvm_is_error_hva(start))
+ return H_STATE;
+
+ end = start + (memslot->npages << PAGE_SHIFT);
+
+ down_write(&kvm->mm->mmap_sem);
+
+ mutex_lock(&kvm->arch.uvmem_lock);
+ vma = find_vma_intersection(kvm->mm, start, end);
+ if (!vma || vma->vm_start > start || vma->vm_end < end) {
+ ret = H_STATE;
+ goto out_unlock;
+ }
+
+ ret = ksm_madvise(vma, vma->vm_start, vma->vm_end,
+ MADV_UNMERGEABLE, &vma->vm_flags);
+ downgrade_write(&kvm->mm->mmap_sem);
+ downgrade = true;
+ if (ret) {
+ ret = H_STATE;
+ goto out_unlock;
+ }
+
+ for (i = 0; i < memslot->npages; i++, ++gfn) {
+ /* skip paged-in pages and shared pages */
+ if (kvmppc_gfn_is_uvmem_pfn(gfn, kvm, NULL) ||
+ kvmppc_gfn_is_uvmem_shared(gfn, kvm))
+ continue;
+
+ start = gfn_to_hva(kvm, gfn);
+ end = start + (1UL << PAGE_SHIFT);
+ ret = kvmppc_svm_migrate_page(vma, start, end,
+ (gfn << PAGE_SHIFT), kvm, PAGE_SHIFT, false);
+
+ if (ret)
+ goto out_unlock;
+ }
+
+out_unlock:
+ mutex_unlock(&kvm->arch.uvmem_lock);
+ if (downgrade)
+ up_read(&kvm->mm->mmap_sem);
+ else
+ up_write(&kvm->mm->mmap_sem);
+ return ret;
+}
+
unsigned long kvmppc_h_svm_init_done(struct kvm *kvm)
{
+ struct kvm_memslots *slots;
+ struct kvm_memory_slot *memslot;
+ int srcu_idx;
+ long ret = H_SUCCESS;
+
if (!(kvm->arch.secure_guest & KVMPPC_SECURE_INIT_START))
return H_UNSUPPORTED;
+ /* migrate any unmoved normal pfn to device pfns*/
+ srcu_idx = srcu_read_lock(&kvm->srcu);
+ slots = kvm_memslots(kvm);
+ kvm_for_each_memslot(memslot, slots) {
+ ret = uv_migrate_mem_slot(kvm, memslot);
+ if (ret) {
+ ret = H_STATE;
+ goto out;
+ }
+ }
+
kvm->arch.secure_guest |= KVMPPC_SECURE_INIT_DONE;
pr_info("LPID %d went secure\n", kvm->arch.lpid);
- return H_SUCCESS;
+
+out:
+ srcu_read_unlock(&kvm->srcu, srcu_idx);
+ return ret;
}
/*
@@ -459,68 +594,6 @@ static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm)
}
/*
- * Alloc a PFN from private device memory pool and copy page from normal
- * memory to secure memory using UV_PAGE_IN uvcall.
- */
-static int kvmppc_svm_page_in(struct vm_area_struct *vma, unsigned long start,
- unsigned long end, unsigned long gpa, struct kvm *kvm,
- unsigned long page_shift, bool *downgrade)
-{
- unsigned long src_pfn, dst_pfn = 0;
- struct migrate_vma mig;
- struct page *spage;
- unsigned long pfn;
- struct page *dpage;
- int ret = 0;
-
- memset(&mig, 0, sizeof(mig));
- mig.vma = vma;
- mig.start = start;
- mig.end = end;
- mig.src = &src_pfn;
- mig.dst = &dst_pfn;
-
- /*
- * We come here with mmap_sem write lock held just for
- * ksm_madvise(), otherwise we only need read mmap_sem.
- * Hence downgrade to read lock once ksm_madvise() is done.
- */
- ret = ksm_madvise(vma, vma->vm_start, vma->vm_end,
- MADV_UNMERGEABLE, &vma->vm_flags);
- downgrade_write(&kvm->mm->mmap_sem);
- *downgrade = true;
- if (ret)
- return ret;
-
- ret = migrate_vma_setup(&mig);
- if (ret)
- return ret;
-
- if (!(*mig.src & MIGRATE_PFN_MIGRATE)) {
- ret = -1;
- goto out_finalize;
- }
-
- dpage = kvmppc_uvmem_get_page(gpa, kvm);
- if (!dpage) {
- ret = -1;
- goto out_finalize;
- }
-
- pfn = *mig.src >> MIGRATE_PFN_SHIFT;
- spage = migrate_pfn_to_page(*mig.src);
- if (spage)
- uv_page_in(kvm->arch.lpid, pfn << page_shift, gpa, 0,
- page_shift);
-
- *mig.dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
- migrate_vma_pages(&mig);
-out_finalize:
- migrate_vma_finalize(&mig);
- return ret;
-}
-
-/*
* Shares the page with HV, thus making it a normal page.
*
* - If the page is already secure, then provision a new page and share
@@ -623,11 +696,23 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
if (!vma || vma->vm_start > start || vma->vm_end < end)
goto out_unlock;
- if (!kvmppc_svm_page_in(vma, start, end, gpa, kvm, page_shift,
- &downgrade)) {
- kvmppc_gfn_uvmem_shared(gfn, kvm, false);
- ret = H_SUCCESS;
+ ret = ksm_madvise(vma, vma->vm_start, vma->vm_end,
+ MADV_UNMERGEABLE, &vma->vm_flags);
+ downgrade_write(&kvm->mm->mmap_sem);
+ downgrade = true;
+ if (ret) {
+ ret = H_PARAMETER;
+ goto out_unlock;
}
+
+ ret = H_PARAMETER;
+ if (kvmppc_svm_migrate_page(vma, start, end, gpa, kvm, page_shift,
+ true))
+ goto out_unlock;
+
+ kvmppc_gfn_uvmem_shared(gfn, kvm, false);
+ ret = H_SUCCESS;
+
out_unlock:
mutex_unlock(&kvm->arch.uvmem_lock);
out:
--
1.8.3.1
^ permalink raw reply related
* [PATCH v1 2/4] KVM: PPC: Book3S HV: track shared GFNs of secure VMs
From: Ram Pai @ 2020-05-31 2:27 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev
Cc: ldufour, linuxram, cclaudio, bharata, aneesh.kumar, sukadev,
bauerman, david
In-Reply-To: <1590892071-25549-1-git-send-email-linuxram@us.ibm.com>
During the life of SVM, its GFNs can transition from secure to shared
state and vice-versa. Since the kernel does not track GFNs that are
shared, it is not possible to disambiguate a shared GFN from a GFN whose
PFN has not yet been migrated to a device-PFN.
The ability to identify a shared GFN is needed to skip migrating its PFN
to device PFN. This functionality is leveraged in a subsequent patch.
Add the ability to identify the state of a GFN.
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bharata B Rao <bharata@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Claudio Carvalho <cclaudio@linux.ibm.com>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 6 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 2 +-
arch/powerpc/kvm/book3s_hv.c | 2 +-
arch/powerpc/kvm/book3s_hv_uvmem.c | 115 ++++++++++++++++++++++++++--
4 files changed, 113 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_uvmem.h b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
index 5a9834e..f0c5708 100644
--- a/arch/powerpc/include/asm/kvm_book3s_uvmem.h
+++ b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
@@ -21,7 +21,8 @@ unsigned long kvmppc_h_svm_page_out(struct kvm *kvm,
int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gfn);
unsigned long kvmppc_h_svm_init_abort(struct kvm *kvm);
void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
- struct kvm *kvm, bool skip_page_out);
+ struct kvm *kvm, bool skip_page_out,
+ bool purge_gfn);
#else
static inline int kvmppc_uvmem_init(void)
{
@@ -75,6 +76,7 @@ static inline int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gfn)
static inline void
kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
- struct kvm *kvm, bool skip_page_out) { }
+ struct kvm *kvm, bool skip_page_out,
+ bool purge_gfn) { }
#endif /* CONFIG_PPC_UV */
#endif /* __ASM_KVM_BOOK3S_UVMEM_H__ */
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 803940d..3448459 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -1100,7 +1100,7 @@ void kvmppc_radix_flush_memslot(struct kvm *kvm,
unsigned int shift;
if (kvm->arch.secure_guest & KVMPPC_SECURE_INIT_START)
- kvmppc_uvmem_drop_pages(memslot, kvm, true);
+ kvmppc_uvmem_drop_pages(memslot, kvm, true, false);
if (kvm->arch.secure_guest & KVMPPC_SECURE_INIT_DONE)
return;
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 103d13e..4c62bfe 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5467,7 +5467,7 @@ static int kvmhv_svm_off(struct kvm *kvm)
continue;
kvm_for_each_memslot(memslot, slots) {
- kvmppc_uvmem_drop_pages(memslot, kvm, true);
+ kvmppc_uvmem_drop_pages(memslot, kvm, true, true);
uv_unregister_mem_slot(kvm->arch.lpid, memslot->id);
}
}
diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
index ea4a1f1..2ef1e03 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -99,14 +99,56 @@
static DEFINE_SPINLOCK(kvmppc_uvmem_bitmap_lock);
#define KVMPPC_UVMEM_PFN (1UL << 63)
+#define KVMPPC_UVMEM_SHARED (1UL << 62)
+#define KVMPPC_UVMEM_FLAG_MASK (KVMPPC_UVMEM_PFN | KVMPPC_UVMEM_SHARED)
+#define KVMPPC_UVMEM_PFN_MASK (~KVMPPC_UVMEM_FLAG_MASK)
struct kvmppc_uvmem_slot {
struct list_head list;
unsigned long nr_pfns;
unsigned long base_pfn;
+ /*
+ * pfns array has an entry for each GFN of the memory slot.
+ *
+ * The GFN can be in one of the following states.
+ *
+ * (a) Secure - The GFN is secure. Only Ultravisor can access it.
+ * (b) Shared - The GFN is shared. Both Hypervisor and Ultravisor
+ * can access it.
+ * (c) Normal - The GFN is a normal. Only Hypervisor can access it.
+ *
+ * Secure GFN is associated with a devicePFN. Its pfn[] has
+ * KVMPPC_UVMEM_PFN flag set, and has the value of the device PFN
+ * KVMPPC_UVMEM_SHARED flag unset, and has the value of the device PFN
+ *
+ * Shared GFN is associated with a memoryPFN. Its pfn[] has
+ * KVMPPC_UVMEM_SHARED flag set. But its KVMPPC_UVMEM_PFN is not set,
+ * and there is no PFN value stored.
+ *
+ * Normal GFN is not associated with memoryPFN. Its pfn[] has
+ * KVMPPC_UVMEM_SHARED and KVMPPC_UVMEM_PFN flag unset, and no PFN
+ * value is stored.
+ *
+ * Any other combination of values in pfn[] leads to undefined
+ * behavior.
+ *
+ * Life cycle of a GFN --
+ *
+ * ---------------------------------------------------------
+ * | | Share | Unshare | SVM |slot |
+ * | | | | abort/ |flush |
+ * | | | | terminate | |
+ * ---------------------------------------------------------
+ * | | | | | |
+ * | Secure | Shared | Secure |Normal |Secure |
+ * | | | | | |
+ * | Shared | Shared | Secure |Normal |Shared |
+ * | | | | | |
+ * | Normal | Shared | Secure |Normal |Normal |
+ * ---------------------------------------------------------
+ */
unsigned long *pfns;
};
-
struct kvmppc_uvmem_page_pvt {
struct kvm *kvm;
unsigned long gpa;
@@ -175,7 +217,12 @@ static void kvmppc_uvmem_pfn_remove(unsigned long gfn, struct kvm *kvm)
list_for_each_entry(p, &kvm->arch.uvmem_pfns, list) {
if (gfn >= p->base_pfn && gfn < p->base_pfn + p->nr_pfns) {
- p->pfns[gfn - p->base_pfn] = 0;
+ /*
+ * Reset everything, but keep the KVMPPC_UVMEM_SHARED
+ * flag intact. A gfn continues to be shared or
+ * unshared, with or without an associated device pfn.
+ */
+ p->pfns[gfn - p->base_pfn] &= KVMPPC_UVMEM_SHARED;
return;
}
}
@@ -193,7 +240,7 @@ static bool kvmppc_gfn_is_uvmem_pfn(unsigned long gfn, struct kvm *kvm,
if (p->pfns[index] & KVMPPC_UVMEM_PFN) {
if (uvmem_pfn)
*uvmem_pfn = p->pfns[index] &
- ~KVMPPC_UVMEM_PFN;
+ KVMPPC_UVMEM_PFN_MASK;
return true;
} else
return false;
@@ -202,6 +249,38 @@ static bool kvmppc_gfn_is_uvmem_pfn(unsigned long gfn, struct kvm *kvm,
return false;
}
+static void kvmppc_gfn_uvmem_shared(unsigned long gfn, struct kvm *kvm,
+ bool set)
+{
+ struct kvmppc_uvmem_slot *p;
+
+ list_for_each_entry(p, &kvm->arch.uvmem_pfns, list) {
+ if (gfn >= p->base_pfn && gfn < p->base_pfn + p->nr_pfns) {
+ unsigned long index = gfn - p->base_pfn;
+
+ if (set)
+ p->pfns[index] |= KVMPPC_UVMEM_SHARED;
+ else
+ p->pfns[index] &= ~KVMPPC_UVMEM_SHARED;
+ return;
+ }
+ }
+}
+
+bool kvmppc_gfn_is_uvmem_shared(unsigned long gfn, struct kvm *kvm)
+{
+ struct kvmppc_uvmem_slot *p;
+
+ list_for_each_entry(p, &kvm->arch.uvmem_pfns, list) {
+ if (gfn >= p->base_pfn && gfn < p->base_pfn + p->nr_pfns) {
+ unsigned long index = gfn - p->base_pfn;
+
+ return (p->pfns[index] & KVMPPC_UVMEM_SHARED);
+ }
+ }
+ return false;
+}
+
unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
{
struct kvm_memslots *slots;
@@ -256,9 +335,13 @@ unsigned long kvmppc_h_svm_init_done(struct kvm *kvm)
* is HV side fault on these pages. Next we *get* these pages, forcing
* fault on them, do fault time migration to replace the device PTEs in
* QEMU page table with normal PTEs from newly allocated pages.
+ *
+ * if @purge_gfn is set, cleanup any information related to each of
+ * the GFNs associated with this memory slot.
*/
void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
- struct kvm *kvm, bool skip_page_out)
+ struct kvm *kvm, bool skip_page_out,
+ bool purge_gfn)
{
int i;
struct kvmppc_uvmem_page_pvt *pvt;
@@ -269,11 +352,22 @@ void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
struct page *uvmem_page;
mutex_lock(&kvm->arch.uvmem_lock);
+
+ if (purge_gfn) {
+ /*
+ * cleanup the shared status of the GFN here.
+ * Any device PFN associated with the GFN shall
+ * be cleaned up later, in kvmppc_uvmem_page_free()
+ * when the device PFN is actually disassociated
+ * from the GFN.
+ */
+ kvmppc_gfn_uvmem_shared(gfn, kvm, false);
+ }
+
if (!kvmppc_gfn_is_uvmem_pfn(gfn, kvm, &uvmem_pfn)) {
mutex_unlock(&kvm->arch.uvmem_lock);
continue;
}
-
uvmem_page = pfn_to_page(uvmem_pfn);
pvt = uvmem_page->zone_device_data;
pvt->skip_page_out = skip_page_out;
@@ -304,7 +398,7 @@ unsigned long kvmppc_h_svm_init_abort(struct kvm *kvm)
srcu_idx = srcu_read_lock(&kvm->srcu);
kvm_for_each_memslot(memslot, kvm_memslots(kvm))
- kvmppc_uvmem_drop_pages(memslot, kvm, false);
+ kvmppc_uvmem_drop_pages(memslot, kvm, false, true);
srcu_read_unlock(&kvm->srcu, srcu_idx);
@@ -470,8 +564,11 @@ static unsigned long kvmppc_share_page(struct kvm *kvm, unsigned long gpa,
goto retry;
}
- if (!uv_page_in(kvm->arch.lpid, pfn << page_shift, gpa, 0, page_shift))
+ if (!uv_page_in(kvm->arch.lpid, pfn << page_shift, gpa, 0,
+ page_shift)) {
+ kvmppc_gfn_uvmem_shared(gfn, kvm, true);
ret = H_SUCCESS;
+ }
kvm_release_pfn_clean(pfn);
mutex_unlock(&kvm->arch.uvmem_lock);
out:
@@ -527,8 +624,10 @@ unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
goto out_unlock;
if (!kvmppc_svm_page_in(vma, start, end, gpa, kvm, page_shift,
- &downgrade))
+ &downgrade)) {
+ kvmppc_gfn_uvmem_shared(gfn, kvm, false);
ret = H_SUCCESS;
+ }
out_unlock:
mutex_unlock(&kvm->arch.uvmem_lock);
out:
--
1.8.3.1
^ permalink raw reply related
* [PATCH v1 1/4] KVM: PPC: Book3S HV: Fix function definition in book3s_hv_uvmem.c
From: Ram Pai @ 2020-05-31 2:27 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev
Cc: ldufour, linuxram, cclaudio, bharata, aneesh.kumar, sukadev,
bauerman, david
In-Reply-To: <1590892071-25549-1-git-send-email-linuxram@us.ibm.com>
Without this fix, GIT gets confused. It generates incorrect
function context for code changes. Weird, but true.
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Bharata B Rao <bharata@linux.ibm.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Claudio Carvalho <cclaudio@linux.ibm.com>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
index 79b1202..ea4a1f1 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -368,8 +368,7 @@ static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm)
* Alloc a PFN from private device memory pool and copy page from normal
* memory to secure memory using UV_PAGE_IN uvcall.
*/
-static int
-kvmppc_svm_page_in(struct vm_area_struct *vma, unsigned long start,
+static int kvmppc_svm_page_in(struct vm_area_struct *vma, unsigned long start,
unsigned long end, unsigned long gpa, struct kvm *kvm,
unsigned long page_shift, bool *downgrade)
{
@@ -436,8 +435,8 @@ static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm)
* In the former case, uses dev_pagemap_ops.migrate_to_ram handler
* to unmap the device page from QEMU's page tables.
*/
-static unsigned long
-kvmppc_share_page(struct kvm *kvm, unsigned long gpa, unsigned long page_shift)
+static unsigned long kvmppc_share_page(struct kvm *kvm, unsigned long gpa,
+ unsigned long page_shift)
{
int ret = H_PARAMETER;
@@ -486,9 +485,9 @@ static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm)
* H_PAGE_IN_SHARED flag makes the page shared which means that the same
* memory in is visible from both UV and HV.
*/
-unsigned long
-kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
- unsigned long flags, unsigned long page_shift)
+unsigned long kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
+ unsigned long flags,
+ unsigned long page_shift)
{
bool downgrade = false;
unsigned long start, end;
@@ -545,10 +544,10 @@ static struct page *kvmppc_uvmem_get_page(unsigned long gpa, struct kvm *kvm)
* Provision a new page on HV side and copy over the contents
* from secure memory using UV_PAGE_OUT uvcall.
*/
-static int
-kvmppc_svm_page_out(struct vm_area_struct *vma, unsigned long start,
- unsigned long end, unsigned long page_shift,
- struct kvm *kvm, unsigned long gpa)
+static int kvmppc_svm_page_out(struct vm_area_struct *vma,
+ unsigned long start,
+ unsigned long end, unsigned long page_shift,
+ struct kvm *kvm, unsigned long gpa)
{
unsigned long src_pfn, dst_pfn = 0;
struct migrate_vma mig;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v1 0/4] Migrate non-migrated pages of a SVM.
From: Ram Pai @ 2020-05-31 2:27 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev
Cc: ldufour, linuxram, cclaudio, bharata, aneesh.kumar, sukadev,
bauerman, david
This patch series migrates the non-migrate pages of a SVM.
This is required when the UV calls H_SVM_INIT_DONE, and
when a memory-slot is hotplugged to a Secure VM.
Laurent Dufour (1):
KVM: PPC: Book3S HV: migrate hot plugged memory
Ram Pai (3):
KVM: PPC: Book3S HV: Fix function definition in book3s_hv_uvmem.c
KVM: PPC: Book3S HV: track shared GFNs of secure VMs
KVM: PPC: Book3S HV: migrate remaining normal-GFNs to secure-GFNs in
H_SVM_INIT_DONE
Documentation/powerpc/ultravisor.rst | 2 +
arch/powerpc/include/asm/kvm_book3s_uvmem.h | 10 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 2 +-
arch/powerpc/kvm/book3s_hv.c | 13 +-
arch/powerpc/kvm/book3s_hv_uvmem.c | 348 +++++++++++++++++++++-------
5 files changed, 284 insertions(+), 91 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: ppc64le and 32-bit LE userland compatibility
From: Will Springer @ 2020-05-31 0:57 UTC (permalink / raw)
To: Segher Boessenkool, linuxppc-dev
Cc: libc-alpha, eery, daniel, musl, binutils, libc-dev
In-Reply-To: <20200530192212.GG31009@gate.crashing.org>
On Saturday, May 30, 2020 12:22:12 PM PDT Segher Boessenkool wrote:
> The original sysv PowerPC supplement
> http://refspecs.linux-foundation.org/elf/elfspec_ppc.pdf
> supports LE as well, and most powerpcle ports use that. But, the
> big-endian Linux ABI differs in quite a few places, and it of course
> makes a lot better sense if powerpcle-linux follows that.
Right, I should have clarified I was talking about Linux ABIs
specifically.
> What patches did you need? I regularly build >30 cross compilers (on
> both BE and LE hosts; I haven't used 32-bit hosts for a long time, but
> in the past those worked fine as well). I also cross-built
> powerpcle-linux-gcc quite a few times (from powerpc64le, from powerpc64,
> from various x86).
There was just an assumption that LE == powerpc64le in libgo, spotted by
q66 (daniel@ on the CC). I just pushed the patch to [1].
> Almost no project that used 32-bit PowerPC in LE mode has sent patches
> to the upstreams.
Right, but I have heard concerns from at least one person familiar with
the ppc kernel about breaking existing users of this arch-endianness
combo, if any. It seems likely that none of those use upstream, though ^^;
> The ABI says long longs are passed in the same order in registers as it
> would be in memory; so the high part and the low part are swapped
> between BE and LE. Which registers make up a pair is exactly the same
> between the two. (You can verify this with an existing powerpcle-*
> compiler, too; I did, and we implement it correctly as far as I can
> see).
I'll give it a closer look. This is my first time poking at this sort of
thing in depth, so excuse my unfamiliarity!
> A huge factor in having good GCC support for powerpcle-linux (or
> anything else) is someone needs to regularly test it, and share test
> results with us (via gcc-testresults@). Hint hint hint :-)
>
> That way we know it is in good shape, know when we are regressing it,
> know there is interest in it.
Once I have more of a bootstrapped userland than a barely-functional
cross chroot, I'll get back to you on that :)
> gl;hf,
>
>
> Segher
Thanks,
Will [she/her]
[1]: https://github.com/Skirmisher/void-packages/blob/master/srcpkgs/gcc/patches/libgo-ppcle.patch
^ permalink raw reply
* Re: [musl] ppc64le and 32-bit LE userland compatibility
From: Will Springer @ 2020-05-30 22:56 UTC (permalink / raw)
To: Rich Felker, linuxppc-dev
Cc: libc-alpha, eery, daniel, musl, binutils, libc-dev
In-Reply-To: <20200529192426.GM1079@brightrain.aerifal.cx>
On Friday, May 29, 2020 12:24:27 PM PDT Rich Felker wrote:
> The argument passing for pread/pwrite is historically a mess and
> differs between archs. musl has a dedicated macro that archs can
> define to override it. But it looks like it should match regardless of
> BE vs LE, and musl already defines it for powerpc with the default
> definition, adding a zero arg to start on an even arg-slot index,
> which is an odd register (since ppc32 args start with an odd one, r3).
>
> > [6]:
> > https://gist.github.com/Skirmisher/02891c1a8cafa0ff18b2460933ef4f3c
> I don't think this is correct, but I'm confused about where it's
> getting messed up because it looks like it should already be right.
Hmm, interesting. Will have to go back to it I guess...
> > This was enough to fix up the `file` bug. I'm no seasoned kernel
> > hacker, though, and there is still concern over the right way to
> > approach this, whether it should live in the kernel or libc, etc.
> > Frankly, I don't know the ABI structure enough to understand why the
> > register padding has to be different in this case, or what
> > lower-level component is responsible for it.. For comparison, I had a
> > look at the mips tree, since it's bi-endian and has a similar 32/64
> > situation. There is a macro conditional upon endianness that is
> > responsible for munging long longs; it uses __MIPSEB__ and __MIPSEL__
> > instead of an if/else on the generic __LITTLE_ENDIAN__. Not sure what
> > to make of that. (It also simply swaps registers for LE, unlike what
> > I did for ppc.)
> Indeed the problem is probably that you need to swap registers for LE,
> not remove the padding slot. Did you check what happens if you pass a
> value larger than 32 bits?
>
> If so, the right way to fix this on the kernel side would be to
> construct the value as a union rather than by bitwise ops so it's
> endian-agnostic:
>
> (union { u32 parts[2]; u64 val; }){{ arg1, arg2 }}.val
>
> But the kernel folks might prefer endian ifdefs for some odd reason...
You are right, this does seem odd considering what the other archs do.
It's quite possible I made a silly mistake, of course...
I haven't tested with values outside the 32-bit range yet; again, this is
new territory for me, so I haven't exactly done exhaustive tests on
everything. I'll give it a closer look.
> > Also worth noting is the one other outstanding bug, where the
> > time-related syscalls in the 32-bit vDSO seem to return garbage. It
> > doesn't look like an endian bug to me, and it doesn't affect standard
> > syscalls (which is why if you run `date` on musl it prints the
> > correct time, unlike on glibc). The vDSO time functions are
> > implemented in ppc asm (arch/powerpc/kernel/vdso32/ gettimeofday.S),
> > and I've never touched the stuff, so if anyone has a clue I'm all
> > ears.
> Not sure about this. Worst-case, just leave it disabled until someone
> finds a fix.
Apparently these asm implementations are being replaced by the generic C
ones [1], so it may be this fixes itself on its own.
Thanks,
Will [she/her]
[1]: https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=173231
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox