* Re: [PATCH 1/9] macintosh/via-macii: Access autopoll_devs when inside lock
From: Finn Thain @ 2020-08-09 23:15 UTC (permalink / raw)
To: Guenter Roeck
Cc: Laurent Vivier, Mark Cave-Ayland, linux-kernel, linux-m68k,
Geert Uytterhoeven, linuxppc-dev, Joshua Thompson
In-Reply-To: <20200809190138.GA133890@roeck-us.net>
On Sun, 9 Aug 2020, Guenter Roeck wrote:
> Hi,
>
> On Sun, Jun 28, 2020 at 02:23:12PM +1000, Finn Thain wrote:
> > The interrupt handler should be excluded when accessing the
> > autopoll_devs variable.
> >
>
> I am quite baffled by this patch. Other than adding an unnecessary lock
> / unlock sequence,
The new lock/unlock sequence means that the expression (autopoll_devs &&
!current_req) can be understood to be atomic. That makes it easier for me
to follow (being that both variables are shared state).
> accessing a variable (which is derived from another variable) from
> inside or outside a lock does not make a difference. If autopoll_devs =
> devs & 0xfffe is 0 inside the lock, it will just as much be 0 outside
> the lock, and vice versa.
>
> Can you explain this in some more detail ? Not that is matters much
> since the change already made it into mainline, but I would like to
> understand what if anything I am missing here.
>
I think the new code is more readable and is obviously free of race
conditions. It's not obvious to me why the old code was free of race
conditions but if you can easily establish that by inspection then you are
a better auditor than I am. Regardless, I'll stick with "Keep It Simple,
Stupid".
^ permalink raw reply
* Re: [PATCH 2/9] macintosh/via-macii: Poll the device most likely to respond
From: Finn Thain @ 2020-08-09 22:58 UTC (permalink / raw)
To: Guenter Roeck
Cc: Laurent Vivier, Mark Cave-Ayland, linux-kernel, linux-m68k,
Geert Uytterhoeven, linuxppc-dev, Joshua Thompson
In-Reply-To: <20200809185541.GA133779@roeck-us.net>
On Sun, 9 Aug 2020, Guenter Roeck wrote:
> Hi,
>
> On Sun, Jun 28, 2020 at 02:23:12PM +1000, Finn Thain wrote:
> > Poll the most recently polled device by default, rather than the lowest
> > device address that happens to be enabled in autopoll_devs. This improves
> > input latency. Re-use macii_queue_poll() rather than duplicate that logic.
> > This eliminates a static struct and function.
> >
> > Fixes: d95fd5fce88f0 ("m68k: Mac II ADB fixes") # v5.0+
> > Tested-by: Stan Johnson <userm57@yahoo.com>
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
>
> With this patch applied, the qemu "q800" emulation no longer works and
> is stuck in early boot. Any idea why that might be the case, and/or how
> to debug it ?
>
The problem you're seeing was mentioned in the cover letter,
https://lore.kernel.org/linux-m68k/cover.1593318192.git.fthain@telegraphics.com.au/
Since this series was merged, Linus' tree is no longer compatible with
long-standing QEMU bugs.
The best way to fix this is to upgrade QEMU (latest is 5.1.0-rc3). Or use
the serial console instead of the framebuffer console.
I regret the inconvenience but the alternative was worse: adding code to
Linux to get compatibility with QEMU bugs (which were added to QEMU due to
Linux bugs).
My main concern is correct operation on actual hardware, as always. But
some QEMU developers are working on support for operating systems besides
Linux.
Therefore, I believe that both QEMU and Linux should aim for compatibility
with actual hardware and not bug compatibility with each other.
^ permalink raw reply
* Re: [PATCH 1/9] macintosh/via-macii: Access autopoll_devs when inside lock
From: Guenter Roeck @ 2020-08-09 19:01 UTC (permalink / raw)
To: Finn Thain
Cc: Laurent Vivier, Mark Cave-Ayland, linux-kernel, linux-m68k,
Geert Uytterhoeven, linuxppc-dev, Joshua Thompson
In-Reply-To: <5952dd8a9bc9de90f1acc4790c51dd42b4c98065.1593318192.git.fthain@telegraphics.com.au>
Hi,
On Sun, Jun 28, 2020 at 02:23:12PM +1000, Finn Thain wrote:
> The interrupt handler should be excluded when accessing the autopoll_devs
> variable.
>
I am quite baffled by this patch. Other than adding an unnecessary lock /
unlock sequence, accessing a variable (which is derived from another
variable) from inside or outside a lock does not make a difference.
If autopoll_devs = devs & 0xfffe is 0 inside the lock, it will just
as much be 0 outside the lock, and vice versa.
Can you explain this in some more detail ? Not that is matters much since
the change already made it into mainline, but I would like to understand
what if anything I am missing here.
Thanks,
Guenter
> Fixes: d95fd5fce88f0 ("m68k: Mac II ADB fixes") # v5.0+
> Tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
> drivers/macintosh/via-macii.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
> index ac824d7b2dcfc..6aa903529570d 100644
> --- a/drivers/macintosh/via-macii.c
> +++ b/drivers/macintosh/via-macii.c
> @@ -270,15 +270,12 @@ static int macii_autopoll(int devs)
> unsigned long flags;
> int err = 0;
>
> + local_irq_save(flags);
> +
> /* bit 1 == device 1, and so on. */
> autopoll_devs = devs & 0xFFFE;
>
> - if (!autopoll_devs)
> - return 0;
> -
> - local_irq_save(flags);
> -
> - if (current_req == NULL) {
> + if (autopoll_devs && !current_req) {
> /* Send a Talk Reg 0. The controller will repeatedly transmit
> * this as long as it is idle.
> */
^ permalink raw reply
* Re: [PATCH 2/9] macintosh/via-macii: Poll the device most likely to respond
From: Guenter Roeck @ 2020-08-09 18:55 UTC (permalink / raw)
To: Finn Thain
Cc: Laurent Vivier, Mark Cave-Ayland, linux-kernel, linux-m68k,
Geert Uytterhoeven, linuxppc-dev, Joshua Thompson
In-Reply-To: <5836f80886ebcfbe5be5fb7e0dc49feed6469712.1593318192.git.fthain@telegraphics.com.au>
Hi,
On Sun, Jun 28, 2020 at 02:23:12PM +1000, Finn Thain wrote:
> Poll the most recently polled device by default, rather than the lowest
> device address that happens to be enabled in autopoll_devs. This improves
> input latency. Re-use macii_queue_poll() rather than duplicate that logic.
> This eliminates a static struct and function.
>
> Fixes: d95fd5fce88f0 ("m68k: Mac II ADB fixes") # v5.0+
> Tested-by: Stan Johnson <userm57@yahoo.com>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
With this patch applied, the qemu "q800" emulation no longer works and is stuck
in early boot. Any idea why that might be the case, and/or how to debug it ?
Thanks,
Guenter
> ---
> drivers/macintosh/via-macii.c | 99 +++++++++++++++++++----------------
> 1 file changed, 53 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/macintosh/via-macii.c b/drivers/macintosh/via-macii.c
> index 6aa903529570d..d4f1a65c5f1fd 100644
> --- a/drivers/macintosh/via-macii.c
> +++ b/drivers/macintosh/via-macii.c
> @@ -77,6 +77,10 @@ static volatile unsigned char *via;
> #define ST_ODD 0x20 /* ADB state: odd data byte */
> #define ST_IDLE 0x30 /* ADB state: idle, nothing to send */
>
> +/* ADB command byte structure */
> +#define ADDR_MASK 0xF0
> +#define CMD_MASK 0x0F
> +
> static int macii_init_via(void);
> static void macii_start(void);
> static irqreturn_t macii_interrupt(int irq, void *arg);
> @@ -117,7 +121,8 @@ static int reply_len; /* number of bytes received in reply_buf or req->reply */
> static int status; /* VIA's ADB status bits captured upon interrupt */
> static int last_status; /* status bits as at previous interrupt */
> static int srq_asserted; /* have to poll for the device that asserted it */
> -static int command_byte; /* the most recent command byte transmitted */
> +static u8 last_cmd; /* the most recent command byte transmitted */
> +static u8 last_poll_cmd; /* the most recent Talk R0 command byte transmitted */
> static int autopoll_devs; /* bits set are device addresses to be polled */
>
> /* Check for MacII style ADB */
> @@ -179,35 +184,49 @@ static int macii_init_via(void)
> /* Send an ADB poll (Talk Register 0 command prepended to the request queue) */
> static void macii_queue_poll(void)
> {
> - /* No point polling the active device as it will never assert SRQ, so
> - * poll the next device in the autopoll list. This could leave us
> - * stuck in a polling loop if an unprobed device is asserting SRQ.
> - * In theory, that could only happen if a device was plugged in after
> - * probing started. Unplugging it again will break the cycle.
> - * (Simply polling the next higher device often ends up polling almost
> - * every device (after wrapping around), which takes too long.)
> - */
> - int device_mask;
> - int next_device;
> static struct adb_request req;
> + unsigned char poll_command;
> + unsigned int poll_addr;
>
> + /* This only polls devices in the autopoll list, which assumes that
> + * unprobed devices never assert SRQ. That could happen if a device was
> + * plugged in after the adb bus scan. Unplugging it again will resolve
> + * the problem. This behaviour is similar to MacOS.
> + */
> if (!autopoll_devs)
> return;
>
> - device_mask = (1 << (((command_byte & 0xF0) >> 4) + 1)) - 1;
> - if (autopoll_devs & ~device_mask)
> - next_device = ffs(autopoll_devs & ~device_mask) - 1;
> - else
> - next_device = ffs(autopoll_devs) - 1;
> + /* The device most recently polled may not be the best device to poll
> + * right now. Some other device(s) may have signalled SRQ (the active
> + * device won't do that). Or the autopoll list may have been changed.
> + * Try polling the next higher address.
> + */
> + poll_addr = (last_poll_cmd & ADDR_MASK) >> 4;
> + if ((srq_asserted && last_cmd == last_poll_cmd) ||
> + !(autopoll_devs & (1 << poll_addr))) {
> + unsigned int higher_devs;
> +
> + higher_devs = autopoll_devs & -(1 << (poll_addr + 1));
> + poll_addr = ffs(higher_devs ? higher_devs : autopoll_devs) - 1;
> + }
>
> - adb_request(&req, NULL, ADBREQ_NOSEND, 1, ADB_READREG(next_device, 0));
> + /* Send a Talk Register 0 command */
> + poll_command = ADB_READREG(poll_addr, 0);
> +
> + /* No need to repeat this Talk command. The transceiver will do that
> + * as long as it is idle.
> + */
> + if (poll_command == last_cmd)
> + return;
> +
> + adb_request(&req, NULL, ADBREQ_NOSEND, 1, poll_command);
>
> req.sent = 0;
> req.complete = 0;
> req.reply_len = 0;
> req.next = current_req;
>
> - if (current_req != NULL) {
> + if (WARN_ON(current_req)) {
> current_req = &req;
> } else {
> current_req = &req;
> @@ -266,37 +285,22 @@ static int macii_write(struct adb_request *req)
> /* Start auto-polling */
> static int macii_autopoll(int devs)
> {
> - static struct adb_request req;
> unsigned long flags;
> - int err = 0;
>
> local_irq_save(flags);
>
> /* bit 1 == device 1, and so on. */
> autopoll_devs = devs & 0xFFFE;
>
> - if (autopoll_devs && !current_req) {
> - /* Send a Talk Reg 0. The controller will repeatedly transmit
> - * this as long as it is idle.
> - */
> - adb_request(&req, NULL, ADBREQ_NOSEND, 1,
> - ADB_READREG(ffs(autopoll_devs) - 1, 0));
> - err = macii_write(&req);
> + if (!current_req) {
> + macii_queue_poll();
> + if (current_req && macii_state == idle)
> + macii_start();
> }
>
> local_irq_restore(flags);
> - return err;
> -}
>
> -static inline int need_autopoll(void)
> -{
> - /* Was the last command Talk Reg 0
> - * and is the target on the autopoll list?
> - */
> - if ((command_byte & 0x0F) == 0x0C &&
> - ((1 << ((command_byte & 0xF0) >> 4)) & autopoll_devs))
> - return 0;
> - return 1;
> + return 0;
> }
>
> /* Prod the chip without interrupts */
> @@ -333,7 +337,12 @@ static void macii_start(void)
> */
>
> /* store command byte */
> - command_byte = req->data[1];
> + last_cmd = req->data[1];
> +
> + /* If this is a Talk Register 0 command, store the command byte */
> + if ((last_cmd & CMD_MASK) == ADB_READREG(0, 0))
> + last_poll_cmd = last_cmd;
> +
> /* Output mode */
> via[ACR] |= SR_OUT;
> /* Load data */
> @@ -424,10 +433,11 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
> if (req->done)
> (*req->done)(req);
>
> - if (current_req)
> + if (!current_req)
> + macii_queue_poll();
> +
> + if (current_req && macii_state == idle)
> macii_start();
> - else if (need_autopoll())
> - macii_autopoll(autopoll_devs);
> }
>
> if (macii_state == idle) {
> @@ -507,14 +517,11 @@ static irqreturn_t macii_interrupt(int irq, void *arg)
>
> macii_state = idle;
>
> - /* SRQ seen before, initiate poll now */
> - if (srq_asserted)
> + if (!current_req)
> macii_queue_poll();
>
> if (current_req)
> macii_start();
> - else if (need_autopoll())
> - macii_autopoll(autopoll_devs);
>
> if (macii_state == idle)
> via[B] = (via[B] & ~ST_MASK) | ST_IDLE;
> --
> 2.26.2
>
^ permalink raw reply
* Re: [RFC PATCH 1/2] powerpc/numa: Introduce logical numa id
From: Aneesh Kumar K.V @ 2020-08-09 18:40 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev, Srikar Dronamraju
In-Reply-To: <6d880a50-09c4-d591-c88c-09fd77512ad3@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> On 8/8/20 2:15 AM, Nathan Lynch wrote:
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>> On 8/7/20 9:54 AM, Nathan Lynch wrote:
>>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>>>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>>>>> index e437a9ac4956..6c659aada55b 100644
>>>>> --- a/arch/powerpc/mm/numa.c
>>>>> +++ b/arch/powerpc/mm/numa.c
>>>>> @@ -221,25 +221,51 @@ static void initialize_distance_lookup_table(int nid,
>>>>> }
>>>>> }
>>>>>
>>>>> +static u32 nid_map[MAX_NUMNODES] = {[0 ... MAX_NUMNODES - 1] = NUMA_NO_NODE};
>>>>
>>>> It's odd to me to use MAX_NUMNODES for this array when it's going to be
>>>> indexed not by Linux's logical node IDs but by the platform-provided
>>>> domain number, which has no relation to MAX_NUMNODES.
>>>
>>>
>>> I didn't want to dynamically allocate this. We could fetch
>>> "ibm,max-associativity-domains" to find the size for that. The current
>>> code do assume firmware group id to not exceed MAX_NUMNODES. Hence kept
>>> the array size to be MAX_NUMNODEs. I do agree that it is confusing. May
>>> be we can do #define MAX_AFFINITY_DOMAIN MAX_NUMNODES?
>>
>> Well, consider:
>>
>> - ibm,max-associativity-domains can change at runtime with LPM. This
>> doesn't happen in practice yet, but we should probably start thinking
>> about how to support that.
>> - The domain numbering isn't clearly specified to have any particular
>> properties such as beginning at zero or a contiguous range.
>>
>> While the current code likely contains assumptions contrary to these
>> points, a change such as this is an opportunity to think about whether
>> those assumptions can be reduced or removed. In particular I think it
>> would be good to gracefully degrade when the number of NUMA affinity
>> domains can exceed MAX_NUMNODES. Using the platform-supplied domain
>> numbers to directly index Linux data structures will make that
>> impossible.
>>
>> So, maybe genradix or even xarray wouldn't actually be overengineering
>> here.
>>
>
> One of the challenges with such a data structure is that we initialize
> the nid_map before the slab is available. This means a memblock based
> allocation and we would end up implementing such a sparse data structure
> ourselves here.
>
> As you mentioned above, since we know that hypervisor as of now limits
> the max affinity domain id below ibm,max-associativity-domains we are
> good with an array-like nid_map we have here. This keeps the code simpler.
>
> This will also allow us to switch to a more sparse data structure as you
> requested here in the future because the main change that is pushed in
> this series is the usage of firmare_group_id_to_nid(). The details of
> the data structure we use to keep track of that mapping are pretty much
> internal to that function.
How about this? This makes it not a direct index. But it do limit the
search to max numa node on the system.
static int domain_id_map[MAX_NUMNODES] = {[0 ... MAX_NUMNODES - 1] = -1 };
static int __affinity_domain_to_nid(int domain_id, int max_nid)
{
int i;
for (i = 0; i < max_nid; i++) {
if (domain_id_map[i] == domain_id)
return i;
}
return NUMA_NO_NODE;
}
int affinity_domain_to_nid(struct affinity_domain *domain)
{
int nid, domain_id;
static int last_nid = 0;
static DEFINE_SPINLOCK(node_id_lock);
domain_id = domain->id;
/*
* For PowerNV we don't change the node id. This helps to avoid
* confusion w.r.t the expected node ids. On pseries, node numbers
* are virtualized. Hence do logical node id for pseries.
*/
if (!firmware_has_feature(FW_FEATURE_LPAR))
return domain_id;
if (domain_id == -1 || last_nid == MAX_NUMNODES)
return NUMA_NO_NODE;
nid = __affinity_domain_to_nid(domain_id, last_nid);
if (nid == NUMA_NO_NODE) {
spin_lock(&node_id_lock);
/* recheck with lock held */
nid = __affinity_domain_to_nid(domain_id, last_nid);
if (nid == NUMA_NO_NODE) {
nid = last_nid++;
domain_id_map[nid] = domain_id;
}
spin_unlock(&node_id_lock);
}
return nid;
}
^ permalink raw reply
* Re: [PASEMI] Nemo board doesn't boot anymore after the commit "powerpc/book3s64/pkeys: Simplify pkey disable branch"
From: Aneesh Kumar K.V @ 2020-08-09 15:03 UTC (permalink / raw)
To: Christian Zigotzky, linuxppc-dev, R.T.Dickinson, Darren Stevens,
mad skateman, Olof Johansson
In-Reply-To: <bdce9b19-a59c-26e3-3fa8-03774e09bca1@linux.ibm.com>
On 8/9/20 8:04 PM, Aneesh Kumar K.V wrote:
> On 8/9/20 7:42 PM, Christian Zigotzky wrote:
>> Hello,
>>
>> The Nemo board (A-EON AmigaOne X1000) [1] doesn't start with the
>> latest Git kernel anymore after the commit "powerpc/book3s64/pkeys:
>> Simplify pkey disable branch" [2].
>>
>> I bisected today [3].
>>
>> Result: powerpc/book3s64/pkeys: Simplify pkey disable branch
>> (a4678d4b477c3d2901f101986ca01406f3b7eaea) [2] is the first bad commit.
>>
>> Unfortunately I wasn't able to revert the first bad commit. The first
>> bad commit depends on many other commits, which unfortunately I don't
>> know. I tried to remove the modifications of the files from the first
>> bad commit but without any success. There are just too many dependencies.
>>
>> Additionally I reverted the commit "selftests/powerpc: Fix pkey
>> syscall redefinitions" [4] and compiled a new kernel but without any
>> success.
>>
>> Could you please check the first bad commit?
>>
>> Thanks,
>> Christian
>>
>
>
> Can you share a successful boot log of the system so that i can double
> check the cpu_feature and mmu_feature reported ? I am looking for
> details similar to below.
>
> [ 0.000000] cpu_features = 0x0001c07f8f5f91a7
> [ 0.000000] possible = 0x0001fbffcf5fb1a7
> [ 0.000000] always = 0x00000003800081a1
> [ 0.000000] cpu_user_features = 0xdc0065c2 0xefe00000
> [ 0.000000] mmu_features = 0x7c006001
> [ 0.000000] firmware_features = 0x0000001fc45bfc57
> [ 0.000000] vmalloc start = 0xc008000000000000
> [ 0.000000] IO start = 0xc00a000000000000
> [ 0.000000] vmemmap start = 0xc00c000000000000
>
>
> IIUC this is P5+? (ISA 2.04). On that pkey should be marked disabled via
>
> static int scan_pkey_feature(void)
> {
> int ret;
> int pkeys_total = 0;
>
> ....
>
> /*
> * Only P7 and above supports SPRN_AMR update with MSR[PR] = 1
> */
> if (!early_cpu_has_feature(CPU_FTR_ARCH_206))
> return 0;
>
>
> }
>
> Can you boot with CONFIG_PPC_MEM_KEYS=n ?
Can you try this change on top of master?
modified arch/powerpc/mm/book3s64/pkeys.c
@@ -215,10 +215,6 @@ void __init pkey_early_init_devtree(void)
pr_info("Enabling pkeys with max key count %d\n", num_pkey);
out:
- /*
- * Setup uamor on boot cpu
- */
- mtspr(SPRN_UAMOR, default_uamor);
return;
}
-aneesh
^ permalink raw reply
* Re: [PASEMI] Nemo board doesn't boot anymore after the commit "powerpc/book3s64/pkeys: Simplify pkey disable branch"
From: Aneesh Kumar K.V @ 2020-08-09 14:34 UTC (permalink / raw)
To: Christian Zigotzky, linuxppc-dev, R.T.Dickinson, Darren Stevens,
mad skateman, Olof Johansson
In-Reply-To: <8f4c1afc-89cf-749b-2b2d-4efa5ef3acff@xenosoft.de>
On 8/9/20 7:42 PM, Christian Zigotzky wrote:
> Hello,
>
> The Nemo board (A-EON AmigaOne X1000) [1] doesn't start with the latest
> Git kernel anymore after the commit "powerpc/book3s64/pkeys: Simplify
> pkey disable branch" [2].
>
> I bisected today [3].
>
> Result: powerpc/book3s64/pkeys: Simplify pkey disable branch
> (a4678d4b477c3d2901f101986ca01406f3b7eaea) [2] is the first bad commit.
>
> Unfortunately I wasn't able to revert the first bad commit. The first
> bad commit depends on many other commits, which unfortunately I don't
> know. I tried to remove the modifications of the files from the first
> bad commit but without any success. There are just too many dependencies.
>
> Additionally I reverted the commit "selftests/powerpc: Fix pkey syscall
> redefinitions" [4] and compiled a new kernel but without any success.
>
> Could you please check the first bad commit?
>
> Thanks,
> Christian
>
Can you share a successful boot log of the system so that i can double
check the cpu_feature and mmu_feature reported ? I am looking for
details similar to below.
[ 0.000000] cpu_features = 0x0001c07f8f5f91a7
[ 0.000000] possible = 0x0001fbffcf5fb1a7
[ 0.000000] always = 0x00000003800081a1
[ 0.000000] cpu_user_features = 0xdc0065c2 0xefe00000
[ 0.000000] mmu_features = 0x7c006001
[ 0.000000] firmware_features = 0x0000001fc45bfc57
[ 0.000000] vmalloc start = 0xc008000000000000
[ 0.000000] IO start = 0xc00a000000000000
[ 0.000000] vmemmap start = 0xc00c000000000000
IIUC this is P5+? (ISA 2.04). On that pkey should be marked disabled via
static int scan_pkey_feature(void)
{
int ret;
int pkeys_total = 0;
....
/*
* Only P7 and above supports SPRN_AMR update with MSR[PR] = 1
*/
if (!early_cpu_has_feature(CPU_FTR_ARCH_206))
return 0;
}
Can you boot with CONFIG_PPC_MEM_KEYS=n ?
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 1/2] powerpc/numa: Introduce logical numa id
From: Aneesh Kumar K.V @ 2020-08-09 14:12 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev, Srikar Dronamraju
In-Reply-To: <87k0yayykz.fsf@linux.ibm.com>
On 8/8/20 2:15 AM, Nathan Lynch wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> On 8/7/20 9:54 AM, Nathan Lynch wrote:
>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
>>>> index e437a9ac4956..6c659aada55b 100644
>>>> --- a/arch/powerpc/mm/numa.c
>>>> +++ b/arch/powerpc/mm/numa.c
>>>> @@ -221,25 +221,51 @@ static void initialize_distance_lookup_table(int nid,
>>>> }
>>>> }
>>>>
>>>> +static u32 nid_map[MAX_NUMNODES] = {[0 ... MAX_NUMNODES - 1] = NUMA_NO_NODE};
>>>
>>> It's odd to me to use MAX_NUMNODES for this array when it's going to be
>>> indexed not by Linux's logical node IDs but by the platform-provided
>>> domain number, which has no relation to MAX_NUMNODES.
>>
>>
>> I didn't want to dynamically allocate this. We could fetch
>> "ibm,max-associativity-domains" to find the size for that. The current
>> code do assume firmware group id to not exceed MAX_NUMNODES. Hence kept
>> the array size to be MAX_NUMNODEs. I do agree that it is confusing. May
>> be we can do #define MAX_AFFINITY_DOMAIN MAX_NUMNODES?
>
> Well, consider:
>
> - ibm,max-associativity-domains can change at runtime with LPM. This
> doesn't happen in practice yet, but we should probably start thinking
> about how to support that.
> - The domain numbering isn't clearly specified to have any particular
> properties such as beginning at zero or a contiguous range.
>
> While the current code likely contains assumptions contrary to these
> points, a change such as this is an opportunity to think about whether
> those assumptions can be reduced or removed. In particular I think it
> would be good to gracefully degrade when the number of NUMA affinity
> domains can exceed MAX_NUMNODES. Using the platform-supplied domain
> numbers to directly index Linux data structures will make that
> impossible.
>
> So, maybe genradix or even xarray wouldn't actually be overengineering
> here.
>
One of the challenges with such a data structure is that we initialize
the nid_map before the slab is available. This means a memblock based
allocation and we would end up implementing such a sparse data structure
ourselves here.
As you mentioned above, since we know that hypervisor as of now limits
the max affinity domain id below ibm,max-associativity-domains we are
good with an array-like nid_map we have here. This keeps the code simpler.
This will also allow us to switch to a more sparse data structure as you
requested here in the future because the main change that is pushed in
this series is the usage of firmare_group_id_to_nid(). The details of
the data structure we use to keep track of that mapping are pretty much
internal to that function.
-aneesh
^ permalink raw reply
* [PASEMI] Nemo board doesn't boot anymore after the commit "powerpc/book3s64/pkeys: Simplify pkey disable branch"
From: Christian Zigotzky @ 2020-08-09 14:12 UTC (permalink / raw)
To: linuxppc-dev, aneesh.kumar, R.T.Dickinson, Darren Stevens,
mad skateman, Olof Johansson
Hello,
The Nemo board (A-EON AmigaOne X1000) [1] doesn't start with the latest
Git kernel anymore after the commit "powerpc/book3s64/pkeys: Simplify
pkey disable branch" [2].
I bisected today [3].
Result: powerpc/book3s64/pkeys: Simplify pkey disable branch
(a4678d4b477c3d2901f101986ca01406f3b7eaea) [2] is the first bad commit.
Unfortunately I wasn't able to revert the first bad commit. The first
bad commit depends on many other commits, which unfortunately I don't
know. I tried to remove the modifications of the files from the first
bad commit but without any success. There are just too many dependencies.
Additionally I reverted the commit "selftests/powerpc: Fix pkey syscall
redefinitions" [4] and compiled a new kernel but without any success.
Could you please check the first bad commit?
Thanks,
Christian
[1] https://en.wikipedia.org/wiki/AmigaOne_X1000
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a4678d4b477c3d2901f101986ca01406f3b7eaea
[3] https://forum.hyperion-entertainment.com/viewtopic.php?p=51340#p51340
[4]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a7aaa2f26bfd932a654706b19859e7adf802bee2
^ permalink raw reply
* Re: linux-next: manual merge of the set_fs tree with the powerpc tree
From: Stephen Rothwell @ 2020-08-09 8:57 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Nathan Lynch, Linux Next Mailing List, PowerPC,
Linux Kernel Mailing List
In-Reply-To: <20200717190931.701ddf08@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]
Hi all,
On Fri, 17 Jul 2020 19:09:31 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the set_fs tree got a conflict in:
>
> arch/powerpc/mm/numa.c
>
> between commit:
>
> c30f931e891e ("powerpc/numa: remove ability to enable topology updates")
>
> from the powerpc tree and commit:
>
> 16a04bde8169 ("proc: switch over direct seq_read method calls to seq_read_iter")
>
> from the set_fs tree.
>
> I fixed it up (the former removed the code updated by the latter, so I
> just did that) and can carry the fix as necessary. This is now fixed as
> far as linux-next is concerned, but any non trivial conflicts should be
> mentioned to your upstream maintainer when your tree is submitted for
> merging. You may also want to consider cooperating with the maintainer
> of the conflicting tree to minimise any particularly complex conflicts.
This is now a conflict between the set_fs tree and Linus' tree.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [powerpc:next-test] BUILD SUCCESS 6fe62ce0c24b06da0b9a8706851e3aaf152fbd72
From: kernel test robot @ 2020-08-08 19:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: 6fe62ce0c24b06da0b9a8706851e3aaf152fbd72 powerpc/smp: Rename ppc_md.cpu_die(void) to ppc_md.cpu_offline_self()
elapsed time: 1799m
configs tested: 100
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
powerpc mpc885_ads_defconfig
arm imx_v6_v7_defconfig
parisc generic-32bit_defconfig
microblaze nommu_defconfig
i386 allyesconfig
sh se7206_defconfig
arm lpc32xx_defconfig
arm ixp4xx_defconfig
m68k apollo_defconfig
mips malta_defconfig
mips maltaup_xpa_defconfig
arm exynos_defconfig
powerpc chrp32_defconfig
mips ip28_defconfig
powerpc64 defconfig
arm mv78xx0_defconfig
sh sh7710voipgw_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc defconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a006-20200808
x86_64 randconfig-a001-20200808
x86_64 randconfig-a002-20200808
x86_64 randconfig-a003-20200808
x86_64 randconfig-a005-20200808
x86_64 randconfig-a004-20200808
i386 randconfig-a005-20200807
i386 randconfig-a004-20200807
i386 randconfig-a001-20200807
i386 randconfig-a002-20200807
i386 randconfig-a003-20200807
i386 randconfig-a006-20200807
i386 randconfig-a004-20200808
i386 randconfig-a005-20200808
i386 randconfig-a001-20200808
i386 randconfig-a003-20200808
i386 randconfig-a002-20200808
i386 randconfig-a006-20200808
x86_64 randconfig-a013-20200807
x86_64 randconfig-a011-20200807
x86_64 randconfig-a012-20200807
x86_64 randconfig-a016-20200807
x86_64 randconfig-a015-20200807
x86_64 randconfig-a014-20200807
i386 randconfig-a011-20200807
i386 randconfig-a012-20200807
i386 randconfig-a013-20200807
i386 randconfig-a015-20200807
i386 randconfig-a014-20200807
i386 randconfig-a016-20200807
i386 randconfig-a011-20200808
i386 randconfig-a014-20200808
i386 randconfig-a015-20200808
i386 randconfig-a013-20200808
i386 randconfig-a012-20200808
i386 randconfig-a016-20200808
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS 1f788123eabf933e828eab9cf8f018ae18faa38f
From: kernel test robot @ 2020-08-08 19:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: 1f788123eabf933e828eab9cf8f018ae18faa38f Automatic merge of 'master', 'next' and 'fixes' (2020-08-08 14:22)
elapsed time: 904m
configs tested: 87
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
ia64 defconfig
m68k sun3x_defconfig
sh sh7770_generic_defconfig
c6x allyesconfig
powerpc mpc885_ads_defconfig
arm imx_v6_v7_defconfig
parisc generic-32bit_defconfig
m68k hp300_defconfig
i386 alldefconfig
mips capcella_defconfig
mips nlm_xlr_defconfig
arm spear3xx_defconfig
sh sh7724_generic_defconfig
ia64 tiger_defconfig
arc tb10x_defconfig
mips rbtx49xx_defconfig
microblaze nommu_defconfig
arm omap2plus_defconfig
arm pxa3xx_defconfig
arc haps_hs_smp_defconfig
arm rpc_defconfig
arm mvebu_v7_defconfig
mips maltasmvp_defconfig
ia64 allmodconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc defconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a006-20200808
x86_64 randconfig-a001-20200808
x86_64 randconfig-a002-20200808
x86_64 randconfig-a003-20200808
x86_64 randconfig-a005-20200808
x86_64 randconfig-a004-20200808
i386 randconfig-a004-20200808
i386 randconfig-a005-20200808
i386 randconfig-a001-20200808
i386 randconfig-a003-20200808
i386 randconfig-a002-20200808
i386 randconfig-a006-20200808
i386 randconfig-a011-20200808
i386 randconfig-a014-20200808
i386 randconfig-a015-20200808
i386 randconfig-a013-20200808
i386 randconfig-a012-20200808
i386 randconfig-a016-20200808
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [EXTERNAL] Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.9-1 tag
From: Aneesh Kumar K.V @ 2020-08-08 14:27 UTC (permalink / raw)
To: Al Viro, Linus Torvalds
Cc: Thadeu Lima de Souza Cascardo, desnesn, Srikar Dronamraju, ego,
aik, jniethe5, bin.meng, psampat, bala24, msuchanek, sathnaga,
Oliver O'Halloran, fthain, Christoph Hellwig,
Linux Kernel Mailing List, equinox, leobras.c, santosh, maddy,
Nayna Jain, YueHaibing, mahesh, Peter Zijlstra, anju,
Geert Uytterhoeven, weiyongjun1, alastair, harish, Waiman Long,
Naveen N. Rao, dyoung, vdronov, nathanl, miltonm, palmerdabbelt,
ajd, Arnd Bergmann, lirongqing, sandipan, kjain, muriloo,
Nick Piggin, Nathan Chancellor, Joe Perches, chris.packham,
Vaibhav Jain, felix, hbathini, Christophe Leroy, atrajeev,
wenxiong, sbobroff, Randy Dunlap, Gustavo A. R. Silva,
sourabhjain, bharata, Tejun Heo, miaoqinglang, Jeremy Kerr,
grandmaster, fbarrat, huntbag, kaloz, linuxppc-dev, Ravi Bangoria,
Bill Wendling
In-Reply-To: <20200807191417.GU1236603@ZenIV.linux.org.uk>
On 8/8/20 12:44 AM, Al Viro wrote:
> On Fri, Aug 07, 2020 at 10:46:13AM -0700, Linus Torvalds wrote:
>> On Fri, Aug 7, 2020 at 6:14 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>>
>>> Just one minor conflict, in a comment in drivers/misc/ocxl/config.c.
>>
>> Well, this morning I merged the ptrace ->regset_get() updates from Al,
>> and that brought in a different conflict.
>>
>> I _think_ I resolved it correctly, but while the new model is fairly
>> readable, the old one sure wasn't, and who knows how messed up my
>> attempt to sort it out was. I don't know the pkey details on powerpc..
>>
>> So I'd appreciate it if both Al and Aneesh Kumar would check that what
>> I did to pkey_get() in arch/powerpc/kernel/ptrace/ptrace-view.c makes
>> sense and works..
>
> Grabbing...
>
> Looks sane and yes, 3 membuf_store() instead of membuf_write() + membuf_store()
> would make sense (might even yield better code). Up to ppc folks...
>
>> Side note - it might have been cleaner to just make it do
>>
>> membuf_store(&to, target->thread.amr);
>> membuf_store(&to, target->thread.iamr);
>> return membuf_store(&to, default_uamor);
That will also allow to get rid of
BUILD_BUG_ON(TSO(amr) + sizeof(unsigned long) != TSO(iamr));
I will followup with a cleanup patch.
>>
>> instead of doing that membuf_write() for the first two ones and then
>> the membuf_store() for the uamor field, but I did what I did to keep
>> the logic as close to what it used to be as possible.
>>
>> If I messed up, I apologize.
>>
>> And if you agree that making it three membuf_store() instead of that
>> odd "depend on the exact order of the thread struct and pick two
>> consecutive values", I'll leave that to you as a separate cleanup.
>>
>> Linus
-aneesh
^ permalink raw reply
* Re: [PATCH 16/22] crypto: ccree - add check for xts input length equal to zero
From: Gilad Ben-Yossef @ 2020-08-08 12:10 UTC (permalink / raw)
To: Andrei Botila
Cc: linux-s390, Andrei Botila, Herbert Xu, x86,
Linux kernel mailing list, linux-arm-kernel,
Linux Crypto Mailing List, linuxppc-dev, David S. Miller,
Linux ARM
In-Reply-To: <20200807162010.18979-17-andrei.botila@oss.nxp.com>
On Fri, Aug 7, 2020 at 7:22 PM Andrei Botila <andrei.botila@oss.nxp.com> wrote:
>
> From: Andrei Botila <andrei.botila@nxp.com>
>
> Standardize the way input lengths equal to 0 are handled in all skcipher
> algorithms. All the algorithms return 0 for input lengths equal to zero.
> This change has implications not only for xts(aes) but also for cts(cbc(aes))
> and cts(cbc(paes)).
>
> Cc: Gilad Ben-Yossef <gilad@benyossef.com>
> Signed-off-by: Andrei Botila <andrei.botila@nxp.com>
> ---
> drivers/crypto/ccree/cc_cipher.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
> index 076669dc1035..112bb8b4dce6 100644
> --- a/drivers/crypto/ccree/cc_cipher.c
> +++ b/drivers/crypto/ccree/cc_cipher.c
> @@ -912,17 +912,18 @@ static int cc_cipher_process(struct skcipher_request *req,
>
> /* STAT_PHASE_0: Init and sanity checks */
>
> - if (validate_data_size(ctx_p, nbytes)) {
> - dev_dbg(dev, "Unsupported data size %d.\n", nbytes);
> - rc = -EINVAL;
> - goto exit_process;
> - }
> if (nbytes == 0) {
> /* No data to process is valid */
> rc = 0;
> goto exit_process;
> }
>
> + if (validate_data_size(ctx_p, nbytes)) {
> + dev_dbg(dev, "Unsupported data size %d.\n", nbytes);
> + rc = -EINVAL;
> + goto exit_process;
> + }
> +
> if (ctx_p->fallback_on) {
> struct skcipher_request *subreq = skcipher_request_ctx(req);
>
> --
> 2.17.1
>
Acked-by: Gilad Ben-Yossef <gilad@benyossef.com>
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.9-1 tag
From: Michael Ellerman @ 2020-08-07 23:27 UTC (permalink / raw)
To: Linus Torvalds, Al Viro, Aneesh Kumar K.V
Cc: Thadeu Lima de Souza Cascardo, desnesn, Srikar Dronamraju, ego,
aik, jniethe5, bin.meng, psampat, bala24, msuchanek, sathnaga,
Oliver O'Halloran, fthain, Christoph Hellwig,
Linux Kernel Mailing List, equinox, leobras.c, santosh, maddy,
Nayna Jain, YueHaibing, mahesh, Peter Zijlstra, anju,
Geert Uytterhoeven, weiyongjun1, alastair, harish, Waiman Long,
Naveen N. Rao, dyoung, vdronov, nathanl, miltonm, palmerdabbelt,
ajd, Arnd Bergmann, lirongqing, sandipan, kjain, muriloo,
Nick Piggin, Nathan Chancellor, Joe Perches, chris.packham,
Vaibhav Jain, felix, hbathini, Christophe Leroy, atrajeev,
wenxiong, sbobroff, Randy Dunlap, Gustavo A. R. Silva,
sourabhjain, bharata, Tejun Heo, miaoqinglang, Jeremy Kerr,
grandmaster, fbarrat, huntbag, kaloz, linuxppc-dev, Ravi Bangoria,
Bill Wendling
In-Reply-To: <CAHk-=wif9A9Y1i1xbie5Qsr7e-YoTpv9O_YSF8NCHWksDPEa2Q@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Fri, Aug 7, 2020 at 6:14 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Just one minor conflict, in a comment in drivers/misc/ocxl/config.c.
>
> Well, this morning I merged the ptrace ->regset_get() updates from Al,
> and that brought in a different conflict.
Ah fooey.
> I _think_ I resolved it correctly, but while the new model is fairly
> readable, the old one sure wasn't, and who knows how messed up my
> attempt to sort it out was. I don't know the pkey details on powerpc..
The old API was horrible, nice to see it gone.
> So I'd appreciate it if both Al and Aneesh Kumar would check that what
> I did to pkey_get() in arch/powerpc/kernel/ptrace/ptrace-view.c makes
> sense and works..
It looks right to me, except it doesn't build due to ret now being unused:
/linux/arch/powerpc/kernel/ptrace/ptrace-view.c: In function ‘pkey_get’:
/linux/arch/powerpc/kernel/ptrace/ptrace-view.c:473:6: error: unused variable ‘ret’ [-Werror=unused-variable]
473 | int ret;
Patch below, do you mind taking it directly?
With that fixed our pkey selftests pass and show the expected values in
those regs.
> Side note - it might have been cleaner to just make it do
>
> membuf_store(&to, target->thread.amr);
> membuf_store(&to, target->thread.iamr);
> return membuf_store(&to, default_uamor);
>
> instead of doing that membuf_write() for the first two ones and then
> the membuf_store() for the uamor field, but I did what I did to keep
> the logic as close to what it used to be as possible.
Yep fair enough.
> If I messed up, I apologize.
>
> And if you agree that making it three membuf_store() instead of that
> odd "depend on the exact order of the thread struct and pick two
> consecutive values", I'll leave that to you as a separate cleanup.
Will do.
cheers
From a280ae69f248a0f87b36170a94c5665ef5353f51 Mon Sep 17 00:00:00 2001
From: Michael Ellerman <mpe@ellerman.id.au>
Date: Sat, 8 Aug 2020 09:12:03 +1000
Subject: [PATCH] powerpc/ptrace: Fix build error in pkey_get()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The merge resolution in commit 25d8d4eecace left ret no longer used,
leading to:
/linux/arch/powerpc/kernel/ptrace/ptrace-view.c: In function ‘pkey_get’:
/linux/arch/powerpc/kernel/ptrace/ptrace-view.c:473:6: error: unused variable ‘ret’
473 | int ret;
Fix it by removing ret.
Fixes: 25d8d4eecace ("Merge tag 'powerpc-5.9-1' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/ptrace/ptrace-view.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c b/arch/powerpc/kernel/ptrace/ptrace-view.c
index 19823a250aa0..7e6478e7ed07 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-view.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-view.c
@@ -470,8 +470,6 @@ static int pkey_active(struct task_struct *target, const struct user_regset *reg
static int pkey_get(struct task_struct *target, const struct user_regset *regset,
struct membuf to)
{
- int ret;
-
BUILD_BUG_ON(TSO(amr) + sizeof(unsigned long) != TSO(iamr));
if (!arch_pkeys_enabled())
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 21/22] crypto: qce - add check for xts input length equal to zero
From: Stanimir Varbanov @ 2020-08-07 17:59 UTC (permalink / raw)
To: Andrei Botila, Herbert Xu, David S. Miller
Cc: linux-s390, Andrei Botila, x86, linux-kernel, linux-arm-kernel,
linux-crypto, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200807162010.18979-22-andrei.botila@oss.nxp.com>
Hi,
Thanks for the patch!
On 8/7/20 7:20 PM, Andrei Botila wrote:
> From: Andrei Botila <andrei.botila@nxp.com>
>
> Standardize the way input lengths equal to 0 are handled in all skcipher
> algorithms. All the algorithms return 0 for input lengths equal to zero.
>
> Signed-off-by: Andrei Botila <andrei.botila@nxp.com>
> ---
> drivers/crypto/qce/skcipher.c | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Stanimir Varbanov <svarbanov@mm-sol.com>
>
> diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
> index 5630c5addd28..887fd4dc9b43 100644
> --- a/drivers/crypto/qce/skcipher.c
> +++ b/drivers/crypto/qce/skcipher.c
> @@ -223,6 +223,9 @@ static int qce_skcipher_crypt(struct skcipher_request *req, int encrypt)
> int keylen;
> int ret;
>
> + if (!req->cryptlen && IS_XTS(rctx->flags))
> + return 0;
> +
> rctx->flags = tmpl->alg_flags;
> rctx->flags |= encrypt ? QCE_ENCRYPT : QCE_DECRYPT;
> keylen = IS_XTS(rctx->flags) ? ctx->enc_keylen >> 1 : ctx->enc_keylen;
>
--
regards,
Stan
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.9-1 tag
From: Al Viro @ 2020-08-07 19:14 UTC (permalink / raw)
To: Linus Torvalds
Cc: Thadeu Lima de Souza Cascardo, desnesn, Srikar Dronamraju, ego,
aik, jniethe5, bin.meng, psampat, bala24, msuchanek, sathnaga,
Oliver O'Halloran, fthain, Christoph Hellwig,
Linux Kernel Mailing List, equinox, leobras.c, santosh, maddy,
Nayna Jain, YueHaibing, mahesh, Peter Zijlstra, anju,
Geert Uytterhoeven, weiyongjun1, alastair, harish, Waiman Long,
Naveen N. Rao, dyoung, vdronov, nathanl, miltonm, palmerdabbelt,
ajd, Arnd Bergmann, lirongqing, sandipan, kjain, muriloo,
Nick Piggin, Nathan Chancellor, Joe Perches, chris.packham,
Vaibhav Jain, felix, hbathini, Christophe Leroy, atrajeev,
wenxiong, sbobroff, Randy Dunlap, Gustavo A. R. Silva,
sourabhjain, bharata, Tejun Heo, miaoqinglang, Jeremy Kerr,
grandmaster, Aneesh Kumar K.V, fbarrat, huntbag, kaloz,
linuxppc-dev, Ravi Bangoria, Bill Wendling
In-Reply-To: <CAHk-=wif9A9Y1i1xbie5Qsr7e-YoTpv9O_YSF8NCHWksDPEa2Q@mail.gmail.com>
On Fri, Aug 07, 2020 at 10:46:13AM -0700, Linus Torvalds wrote:
> On Fri, Aug 7, 2020 at 6:14 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > Just one minor conflict, in a comment in drivers/misc/ocxl/config.c.
>
> Well, this morning I merged the ptrace ->regset_get() updates from Al,
> and that brought in a different conflict.
>
> I _think_ I resolved it correctly, but while the new model is fairly
> readable, the old one sure wasn't, and who knows how messed up my
> attempt to sort it out was. I don't know the pkey details on powerpc..
>
> So I'd appreciate it if both Al and Aneesh Kumar would check that what
> I did to pkey_get() in arch/powerpc/kernel/ptrace/ptrace-view.c makes
> sense and works..
Grabbing...
Looks sane and yes, 3 membuf_store() instead of membuf_write() + membuf_store()
would make sense (might even yield better code). Up to ppc folks...
> Side note - it might have been cleaner to just make it do
>
> membuf_store(&to, target->thread.amr);
> membuf_store(&to, target->thread.iamr);
> return membuf_store(&to, default_uamor);
>
> instead of doing that membuf_write() for the first two ones and then
> the membuf_store() for the uamor field, but I did what I did to keep
> the logic as close to what it used to be as possible.
>
> If I messed up, I apologize.
>
> And if you agree that making it three membuf_store() instead of that
> odd "depend on the exact order of the thread struct and pick two
> consecutive values", I'll leave that to you as a separate cleanup.
>
> Linus
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.9-1 tag
From: pr-tracker-bot @ 2020-08-07 17:47 UTC (permalink / raw)
To: Michael Ellerman
Cc: cascardo, desnesn, gustavoars, srikar, ego, aik, jniethe5,
bin.meng, psampat, bala24, npiggin, sathnaga, lirongqing, hch,
equinox, leobras.c, santosh, maddy, aneesh.kumar, nayna,
yuehaibing, fbarrat, fthain, mahesh, peterz, anju, geert,
weiyongjun1, alastair, harish, longman, naveen.n.rao, msuchanek,
dyoung, vdronov, nathanl, miltonm, palmerdabbelt, ajd, arnd,
kjain, muriloo, tj, chris.packham, vaibhav, felix, natechancellor,
hbathini, christophe.leroy, atrajeev, wenxiong, morbo, sbobroff,
linuxppc-dev, rdunlap, linux-kernel, sourabhjain, bharata,
miaoqinglang, ail.com, jk, grandmaster, huntbag, joe, oohall,
kaloz, Linus Torvalds, ravi.bangoria, sandipan
In-Reply-To: <87h7tey4xq.fsf@mpe.ellerman.id.au>
The pull request you sent on Fri, 07 Aug 2020 23:13:37 +1000:
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.9-1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/25d8d4eecace9de5a6a2193e4df1917afbdd3052
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.9-1 tag
From: Linus Torvalds @ 2020-08-07 17:46 UTC (permalink / raw)
To: Michael Ellerman, Al Viro, Aneesh Kumar K.V
Cc: Thadeu Lima de Souza Cascardo, desnesn, Srikar Dronamraju, ego,
aik, jniethe5, bin.meng, psampat, bala24, msuchanek, sathnaga,
Oliver O'Halloran, fthain, Christoph Hellwig,
Linux Kernel Mailing List, equinox, leobras.c, santosh, maddy,
Nayna Jain, YueHaibing, mahesh, Peter Zijlstra, anju,
Geert Uytterhoeven, weiyongjun1, alastair, harish, Waiman Long,
Naveen N. Rao, dyoung, vdronov, nathanl, miltonm, palmerdabbelt,
ajd, Arnd Bergmann, lirongqing, sandipan, kjain, muriloo,
Nick Piggin, Nathan Chancellor, Joe Perches, chris.packham,
Vaibhav Jain, felix, hbathini, Christophe Leroy, atrajeev,
wenxiong, sbobroff, Randy Dunlap, Gustavo A. R. Silva,
sourabhjain, bharata, Tejun Heo, miaoqinglang, Jeremy Kerr,
grandmaster, fbarrat, huntbag, kaloz, linuxppc-dev, Ravi Bangoria,
Bill Wendling
In-Reply-To: <87h7tey4xq.fsf@mpe.ellerman.id.au>
On Fri, Aug 7, 2020 at 6:14 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Just one minor conflict, in a comment in drivers/misc/ocxl/config.c.
Well, this morning I merged the ptrace ->regset_get() updates from Al,
and that brought in a different conflict.
I _think_ I resolved it correctly, but while the new model is fairly
readable, the old one sure wasn't, and who knows how messed up my
attempt to sort it out was. I don't know the pkey details on powerpc..
So I'd appreciate it if both Al and Aneesh Kumar would check that what
I did to pkey_get() in arch/powerpc/kernel/ptrace/ptrace-view.c makes
sense and works..
Side note - it might have been cleaner to just make it do
membuf_store(&to, target->thread.amr);
membuf_store(&to, target->thread.iamr);
return membuf_store(&to, default_uamor);
instead of doing that membuf_write() for the first two ones and then
the membuf_store() for the uamor field, but I did what I did to keep
the logic as close to what it used to be as possible.
If I messed up, I apologize.
And if you agree that making it three membuf_store() instead of that
odd "depend on the exact order of the thread struct and pick two
consecutive values", I'll leave that to you as a separate cleanup.
Linus
^ permalink raw reply
* [PATCH] arch/powerpc: use simple i2c probe function
From: Stephen Kitt @ 2020-08-07 15:27 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Scott Wood, linuxppc-dev
Cc: Stephen Kitt, linux-i2c, linux-kernel
The i2c probe functions here don't use the id information provided in
their second argument, so the single-parameter i2c probe function
("probe_new") can be used instead.
This avoids scanning the identifier tables during probes.
Signed-off-by: Stephen Kitt <steve@sk2.org>
---
arch/powerpc/platforms/44x/ppc476.c | 5 ++---
arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 4 ++--
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/44x/ppc476.c b/arch/powerpc/platforms/44x/ppc476.c
index cba83eee685c..07f7e3ce67b5 100644
--- a/arch/powerpc/platforms/44x/ppc476.c
+++ b/arch/powerpc/platforms/44x/ppc476.c
@@ -86,8 +86,7 @@ static void __noreturn avr_reset_system(char *cmd)
avr_halt_system(AVR_PWRCTL_RESET);
}
-static int avr_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int avr_probe(struct i2c_client *client)
{
avr_i2c_client = client;
ppc_md.restart = avr_reset_system;
@@ -104,7 +103,7 @@ static struct i2c_driver avr_driver = {
.driver = {
.name = "akebono-avr",
},
- .probe = avr_probe,
+ .probe_new = avr_probe,
.id_table = avr_id,
};
diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 0967bdfb1691..409481016928 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -142,7 +142,7 @@ static int mcu_gpiochip_remove(struct mcu *mcu)
return 0;
}
-static int mcu_probe(struct i2c_client *client, const struct i2c_device_id *id)
+static int mcu_probe(struct i2c_client *client)
{
struct mcu *mcu;
int ret;
@@ -221,7 +221,7 @@ static struct i2c_driver mcu_driver = {
.name = "mcu-mpc8349emitx",
.of_match_table = mcu_of_match_table,
},
- .probe = mcu_probe,
+ .probe_new = mcu_probe,
.remove = mcu_remove,
.id_table = mcu_ids,
};
--
2.25.4
^ permalink raw reply related
* [PATCH 22/22] crypto: vmx - add check for xts input length equal to zero
From: Andrei Botila @ 2020-08-07 16:20 UTC (permalink / raw)
To: Herbert Xu, David S. Miller
Cc: linux-s390, Andrei Botila, Nayna Jain, x86, linux-kernel,
linux-arm-kernel, Paulo Flabiano Smorigo, linux-crypto,
Breno Leitão, Paul Mackerras, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200807162010.18979-1-andrei.botila@oss.nxp.com>
From: Andrei Botila <andrei.botila@nxp.com>
Standardize the way input lengths equal to 0 are handled in all skcipher
algorithms. All the algorithms return 0 for input lengths equal to zero.
Cc: "Breno Leitão" <leitao@debian.org>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: Paulo Flabiano Smorigo <pfsmorigo@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Andrei Botila <andrei.botila@nxp.com>
---
drivers/crypto/vmx/aes_xts.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
index 9fee1b1532a4..33107c9e2656 100644
--- a/drivers/crypto/vmx/aes_xts.c
+++ b/drivers/crypto/vmx/aes_xts.c
@@ -84,6 +84,9 @@ static int p8_aes_xts_crypt(struct skcipher_request *req, int enc)
u8 tweak[AES_BLOCK_SIZE];
int ret;
+ if (!req->cryptlen)
+ return 0;
+
if (req->cryptlen < AES_BLOCK_SIZE)
return -EINVAL;
--
2.17.1
^ permalink raw reply related
* [PATCH 21/22] crypto: qce - add check for xts input length equal to zero
From: Andrei Botila @ 2020-08-07 16:20 UTC (permalink / raw)
To: Herbert Xu, David S. Miller
Cc: linux-s390, Andrei Botila, x86, linux-kernel, linux-arm-kernel,
linux-crypto, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200807162010.18979-1-andrei.botila@oss.nxp.com>
From: Andrei Botila <andrei.botila@nxp.com>
Standardize the way input lengths equal to 0 are handled in all skcipher
algorithms. All the algorithms return 0 for input lengths equal to zero.
Signed-off-by: Andrei Botila <andrei.botila@nxp.com>
---
drivers/crypto/qce/skcipher.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index 5630c5addd28..887fd4dc9b43 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -223,6 +223,9 @@ static int qce_skcipher_crypt(struct skcipher_request *req, int encrypt)
int keylen;
int ret;
+ if (!req->cryptlen && IS_XTS(rctx->flags))
+ return 0;
+
rctx->flags = tmpl->alg_flags;
rctx->flags |= encrypt ? QCE_ENCRYPT : QCE_DECRYPT;
keylen = IS_XTS(rctx->flags) ? ctx->enc_keylen >> 1 : ctx->enc_keylen;
--
2.17.1
^ permalink raw reply related
* [PATCH 20/22] crypto: octeontx - add check for xts input length equal to zero
From: Andrei Botila @ 2020-08-07 16:20 UTC (permalink / raw)
To: Herbert Xu, David S. Miller
Cc: linux-s390, Andrei Botila, Arnaud Ebalard, Boris Brezillon, x86,
linux-kernel, linux-arm-kernel, Srujana Challa, linux-crypto,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200807162010.18979-1-andrei.botila@oss.nxp.com>
From: Andrei Botila <andrei.botila@nxp.com>
Standardize the way input lengths equal to 0 are handled in all skcipher
algorithms. All the algorithms return 0 for input lengths equal to zero.
Cc: Boris Brezillon <bbrezillon@kernel.org>
Cc: Arnaud Ebalard <arno@natisbad.org>
Cc: Srujana Challa <schalla@marvell.com>
Signed-off-by: Andrei Botila <andrei.botila@nxp.com>
---
drivers/crypto/marvell/octeontx/otx_cptvf_algs.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/crypto/marvell/octeontx/otx_cptvf_algs.c b/drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
index 90bb31329d4b..ec13bc3f1766 100644
--- a/drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
+++ b/drivers/crypto/marvell/octeontx/otx_cptvf_algs.c
@@ -340,11 +340,16 @@ static inline int cpt_enc_dec(struct skcipher_request *req, u32 enc)
{
struct crypto_skcipher *stfm = crypto_skcipher_reqtfm(req);
struct otx_cpt_req_ctx *rctx = skcipher_request_ctx(req);
+ struct crypto_tfm *tfm = crypto_skcipher_tfm(stfm);
+ struct otx_cpt_enc_ctx *ctx = crypto_tfm_ctx(tfm);
struct otx_cpt_req_info *req_info = &rctx->cpt_req;
u32 enc_iv_len = crypto_skcipher_ivsize(stfm);
struct pci_dev *pdev;
int status, cpu_num;
+ if (!req->cryptlen && ctx->cipher_type == OTX_CPT_AES_XTS)
+ return 0;
+
/* Validate that request doesn't exceed maximum CPT supported size */
if (req->cryptlen > OTX_CPT_MAX_REQ_SIZE)
return -E2BIG;
--
2.17.1
^ permalink raw reply related
* [PATCH 19/22] crypto: inside-secure - add check for xts input length equal to zero
From: Andrei Botila @ 2020-08-07 16:20 UTC (permalink / raw)
To: Herbert Xu, David S. Miller
Cc: linux-s390, Andrei Botila, Antoine Tenart, x86, linux-kernel,
linux-arm-kernel, linux-crypto, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200807162010.18979-1-andrei.botila@oss.nxp.com>
From: Andrei Botila <andrei.botila@nxp.com>
Standardize the way input lengths equal to 0 are handled in all skcipher
algorithms. All the algorithms return 0 for input lengths equal to zero.
Cc: Antoine Tenart <antoine.tenart@bootlin.com>
Signed-off-by: Andrei Botila <andrei.botila@nxp.com>
---
drivers/crypto/inside-secure/safexcel_cipher.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index 1ac3253b7903..03d06556ea98 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -2533,6 +2533,9 @@ static int safexcel_skcipher_aes_xts_cra_init(struct crypto_tfm *tfm)
static int safexcel_encrypt_xts(struct skcipher_request *req)
{
+ if (!req->cryptlen)
+ return 0;
+
if (req->cryptlen < XTS_BLOCK_SIZE)
return -EINVAL;
return safexcel_queue_req(&req->base, skcipher_request_ctx(req),
@@ -2541,6 +2544,9 @@ static int safexcel_encrypt_xts(struct skcipher_request *req)
static int safexcel_decrypt_xts(struct skcipher_request *req)
{
+ if (!req->cryptlen)
+ return 0;
+
if (req->cryptlen < XTS_BLOCK_SIZE)
return -EINVAL;
return safexcel_queue_req(&req->base, skcipher_request_ctx(req),
--
2.17.1
^ permalink raw reply related
* [PATCH 18/22] crypto: hisilicon/sec - add check for xts input length equal to zero
From: Andrei Botila @ 2020-08-07 16:20 UTC (permalink / raw)
To: Herbert Xu, David S. Miller
Cc: linux-s390, Andrei Botila, x86, linux-kernel, linux-arm-kernel,
linux-crypto, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200807162010.18979-1-andrei.botila@oss.nxp.com>
From: Andrei Botila <andrei.botila@nxp.com>
Standardize the way input lengths equal to 0 are handled in all skcipher
algorithms. All the algorithms return 0 for input lengths equal to zero.
Signed-off-by: Andrei Botila <andrei.botila@nxp.com>
---
drivers/crypto/hisilicon/sec/sec_algs.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/crypto/hisilicon/sec/sec_algs.c b/drivers/crypto/hisilicon/sec/sec_algs.c
index 8ca945ac297e..419ec4f23164 100644
--- a/drivers/crypto/hisilicon/sec/sec_algs.c
+++ b/drivers/crypto/hisilicon/sec/sec_algs.c
@@ -723,6 +723,10 @@ static int sec_alg_skcipher_crypto(struct skcipher_request *skreq,
bool split = skreq->src != skreq->dst;
gfp_t gfp = skreq->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP ? GFP_KERNEL : GFP_ATOMIC;
+ if (!skreq->cryptlen && (ctx->cipher_alg == SEC_C_AES_XTS_128 ||
+ ctx->cipher_alg == SEC_C_AES_XTS_256))
+ return 0;
+
mutex_init(&sec_req->lock);
sec_req->req_base = &skreq->base;
sec_req->err = 0;
--
2.17.1
^ permalink raw reply related
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