Re: [PATCH 05/10] crypto: keembay: ocs-hcu: Fix incorrectly named functions/structs
On Thu, 2021-03-18 at 12:44 +, Lee Jones wrote:
> Fixes the following W=1 kernel build warning(s):
>
> drivers/crypto/keembay/ocs-hcu.c:107: warning: expecting prototype for
> struct ocs_hcu_dma_list. Prototype was for struct ocs_hcu_dma_entry instead
> drivers/crypto/keembay/ocs-hcu.c:127: warning: expecting prototype for
> struct ocs_dma_list. Prototype was for struct ocs_hcu_dma_list instead
> drivers/crypto/keembay/ocs-hcu.c:610: warning: expecting prototype for
> ocs_hcu_digest(). Prototype was for ocs_hcu_hash_update() instead
> drivers/crypto/keembay/ocs-hcu.c:648: warning: expecting prototype for
> ocs_hcu_hash_final(). Prototype was for ocs_hcu_hash_finup() instead
>
> Cc: Daniele Alessandrelli
> Cc: Declan Murphy
> Cc: Herbert Xu
> Cc: "David S. Miller"
> Cc: linux-cry...@vger.kernel.org
> Signed-off-by: Lee Jones
> ---
Reviewed-by: Daniele Alessandrelli
Thanks for the fix!
> drivers/crypto/keembay/ocs-hcu.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/crypto/keembay/ocs-hcu.c
> b/drivers/crypto/keembay/ocs-hcu.c
> index 81eecacf603ad..deb9bd460ee62 100644
> --- a/drivers/crypto/keembay/ocs-hcu.c
> +++ b/drivers/crypto/keembay/ocs-hcu.c
> @@ -93,7 +93,7 @@
> #define OCS_HCU_WAIT_BUSY_TIMEOUT_US 100
>
> /**
> - * struct ocs_hcu_dma_list - An entry in an OCS DMA linked list.
> + * struct ocs_hcu_dma_entry - An entry in an OCS DMA linked list.
> * @src_addr: Source address of the data.
> * @src_len: Length of data to be fetched.
> * @nxt_desc: Next descriptor to fetch.
> @@ -107,7 +107,7 @@ struct ocs_hcu_dma_entry {
> };
>
> /**
> - * struct ocs_dma_list - OCS-specific DMA linked list.
> + * struct ocs_hcu_dma_list - OCS-specific DMA linked list.
> * @head:The head of the list (points to the array backing the list).
> * @tail:The current tail of the list; NULL if the list is empty.
> * @dma_addr:The DMA address of @head (i.e., the DMA address of the
> backing
> @@ -597,7 +597,7 @@ int ocs_hcu_hash_init(struct ocs_hcu_hash_ctx *ctx, enum
> ocs_hcu_algo algo)
> }
>
> /**
> - * ocs_hcu_digest() - Perform a hashing iteration.
> + * ocs_hcu_hash_update() - Perform a hashing iteration.
> * @hcu_dev: The OCS HCU device to use.
> * @ctx: The OCS HCU hashing context.
> * @dma_list:The OCS DMA list mapping the input data to process.
> @@ -632,7 +632,7 @@ int ocs_hcu_hash_update(struct ocs_hcu_dev *hcu_dev,
> }
>
> /**
> - * ocs_hcu_hash_final() - Update and finalize hash computation.
> + * ocs_hcu_hash_finup() - Update and finalize hash computation.
> * @hcu_dev: The OCS HCU device to use.
> * @ctx: The OCS HCU hashing context.
> * @dma_list:The OCS DMA list mapping the input data to process.
Re: [RESEND PATCH 00/10] arm64: dts: intel: socfpga: minor cleanups
On Mon, 2021-03-08 at 17:22 +0100, Krzysztof Kozlowski wrote: > Hi Arnd and Olof, > > This is just a resend of previous patch. Since I did not get replies > from Intel maintainers, I assume this could go via soc tree directly. I think the to/cc list is missing Dinh, the socfpga maintainer: Dinh Nguyen I already acked the only patch I could review, i.e.: dt-bindings: arm: intel,keembay: limit the dtschema to root node Regards, Daniele > > Best regards, > Krzysztof > > > Krzysztof Kozlowski (10): > dt-bindings: arm: intel,keembay: limit the dtschema to root node > arm64: dts: intel: socfpga: override clocks by label > arm64: dts: intel: socfpga_agilex: move clocks out of soc node > arm64: dts: intel: socfpga_agilex: move timer out of soc node > arm64: dts: intel: socfpga_agilex: remove default status=okay > arm64: dts: intel: socfpga_agilex: move usbphy out of soc node > arm64: dts: intel: socfpga_agilex: use defined for GIC interrupts > arm64: dts: intel: socfpga_agilex: align node names with dtschema > arm64: dts: intel: socfpga_agilex_socdk: align LED node names with > dtschema > arm64: dts: intel: socfpga_agilex_socdk_nand: align LED node names > with dtschema > > .../bindings/arm/intel,keembay.yaml | 2 + > arch/arm64/boot/dts/intel/socfpga_agilex.dtsi | 222 ++ > > .../boot/dts/intel/socfpga_agilex_socdk.dts | 18 +- > .../dts/intel/socfpga_agilex_socdk_nand.dts | 18 +- > .../boot/dts/intel/socfpga_n5x_socdk.dts | 12 +- > 5 files changed, 144 insertions(+), 128 deletions(-) >
Re: [PATCH 05/10] crypto: keembay: ocs-hcu: Fix incorrectly named functions/structs
Hi Lee, Thanks for the patch. On Wed, 2021-03-03 at 14:34 +, Lee Jones wrote: > Fixes the following W=1 kernel build warning(s): > > drivers/crypto/keembay/ocs-hcu.c:107: warning: expecting prototype for > struct ocs_hcu_dma_list. Prototype was for struct ocs_hcu_dma_entry instead > drivers/crypto/keembay/ocs-hcu.c:127: warning: expecting prototype for > struct ocs_dma_list. Prototype was for struct ocs_hcu_dma_list instead I don't see the fix for this in the diff. Am I missing something? > drivers/crypto/keembay/ocs-hcu.c:610: warning: expecting prototype for > ocs_hcu_digest(). Prototype was for ocs_hcu_hash_update() instead > drivers/crypto/keembay/ocs-hcu.c:648: warning: expecting prototype for > ocs_hcu_hash_final(). Prototype was for ocs_hcu_hash_finup() instead > > Cc: Daniele Alessandrelli > Cc: Declan Murphy > Cc: Herbert Xu > Cc: "David S. Miller" > Cc: linux-cry...@vger.kernel.org > Signed-off-by: Lee Jones > --- > drivers/crypto/keembay/ocs-hcu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/crypto/keembay/ocs-hcu.c > b/drivers/crypto/keembay/ocs-hcu.c > index 81eecacf603ad..d522757855fb0 100644 > --- a/drivers/crypto/keembay/ocs-hcu.c > +++ b/drivers/crypto/keembay/ocs-hcu.c > @@ -93,7 +93,7 @@ > #define OCS_HCU_WAIT_BUSY_TIMEOUT_US 100 > > /** > - * struct ocs_hcu_dma_list - An entry in an OCS DMA linked list. > + * struct ocs_hcu_dma_entry - An entry in an OCS DMA linked list. > * @src_addr: Source address of the data. > * @src_len: Length of data to be fetched. > * @nxt_desc: Next descriptor to fetch. > @@ -597,7 +597,7 @@ int ocs_hcu_hash_init(struct ocs_hcu_hash_ctx *ctx, enum > ocs_hcu_algo algo) > } > > /** > - * ocs_hcu_digest() - Perform a hashing iteration. > + * ocs_hcu_hash_update() - Perform a hashing iteration. > * @hcu_dev: The OCS HCU device to use. > * @ctx: The OCS HCU hashing context. > * @dma_list:The OCS DMA list mapping the input data to process. > @@ -632,7 +632,7 @@ int ocs_hcu_hash_update(struct ocs_hcu_dev *hcu_dev, > } > > /** > - * ocs_hcu_hash_final() - Update and finalize hash computation. > + * ocs_hcu_hash_finup() - Update and finalize hash computation. > * @hcu_dev: The OCS HCU device to use. > * @ctx: The OCS HCU hashing context. > * @dma_list:The OCS DMA list mapping the input data to process.
Re: [PATCH v6 03/34] mailbox: vpu-ipc-mailbox: Add support for Intel VPU IPC mailbox
Hi Jassi, Thank you very much for your feedback. On Sun, 2021-02-14 at 22:54 -0600, Jassi Brar wrote: > IIUIC, maybe the solution is simpler What if we set txdone_poll. > Always return success in send_data(). And check if we overflew the > fifo in last_tx_done(). If we did overflow, try to rewrite the data > and check again. Return true, if not overflew this time, otherwise > return false so that mailbox api can ask us to try again in next > last_tx_done(). This way we can do away with the tasklet and, more > importantly, avoid send_data() failures and retries on clients' part. That's a clever solution to avoid the tasklet. The only issue for us is the automatic TX retry from the controller. I understand that's generally a desirable feature, but in our case we'd like the client to have full control on re-transmission attempts. That's because some of our data is time-sensitive. For instance, when we process frames from a video stream we prefer dropping a frame rather than re-transmitting it and delaying the processing of the rest. Now, I understand that the client can set the 'tx_block' and 'tx_tout' channel fields to specify how long it wishes to wait, but the problem is that our (single) channel is shared between multiple applications having different timing requirements. That's why we prefer to let applications deal we re-transmissions. Given the above, do you think it's reasonable to leave the implementation as it is now? (from initial analysis, the tasklet doesn't seem to affect the performance of our use cases significantly, so we are fine with it)
Re: [PATCH 02/11] dt-bindings: arm: intel,keembay: limit the dtschema to root node
Hi Krzysztof, Thanks for the fix. On Wed, 2021-02-10 at 18:18 +0100, Krzysztof Kozlowski wrote: > The check for the board compatible should be limited only to the root > node. Any other nodes with such compatible are not part of this schema > and should not match. > > Signed-off-by: Krzysztof Kozlowski > --- > Documentation/devicetree/bindings/arm/intel,keembay.yaml | 2 ++ > 1 file changed, 2 insertions(+) Acked-by: Daniele Alessandrelli > > diff --git a/Documentation/devicetree/bindings/arm/intel,keembay.yaml > b/Documentation/devicetree/bindings/arm/intel,keembay.yaml > index 69cd30872928..107e686ab207 100644 > --- a/Documentation/devicetree/bindings/arm/intel,keembay.yaml > +++ b/Documentation/devicetree/bindings/arm/intel,keembay.yaml > @@ -11,6 +11,8 @@ maintainers: >- Daniele Alessandrelli > > properties: > + $nodename: > +const: '/' >compatible: > items: >- enum:
Re: [PATCH 06/20] crypto: keembay: ocs-hcu: Fix incorrectly named functions/structs
On Thu, 2021-02-04 at 18:55 +, Lee Jones wrote: > On Thu, 04 Feb 2021, Alessandrelli, Daniele wrote: > > > On Thu, 2021-02-04 at 11:09 +, Lee Jones wrote: > > > Fixes the following W=1 kernel build warning(s): > > > > > > drivers/crypto/keembay/ocs-hcu.c:107: warning: expecting prototype for > > > struct ocs_hcu_dma_list. Prototype was for struct ocs_hcu_dma_entry > > > instead > > > drivers/crypto/keembay/ocs-hcu.c:127: warning: expecting prototype for > > > struct ocs_dma_list. Prototype was for struct ocs_hcu_dma_list instead > > > drivers/crypto/keembay/ocs-hcu.c:610: warning: expecting prototype for > > > ocs_hcu_digest(). Prototype was for ocs_hcu_hash_update() instead > > > drivers/crypto/keembay/ocs-hcu.c:648: warning: expecting prototype for > > > ocs_hcu_hash_final(). Prototype was for ocs_hcu_hash_finup() instead > > > > > > Cc: Daniele Alessandrelli > > > Cc: Declan Murphy > > > Cc: Herbert Xu > > > Cc: "David S. Miller" > > > Cc: linux-cry...@vger.kernel.org > > > Signed-off-by: Lee Jones > > > --- > > > > Acked-by: Daniele Alessandrelli > > > > > > Thanks for fixing these. > > > > For some reason, if the issues are there, I don't get those warnings > > when compiling with W=1; the command I run is: > > > >make CROSS_COMPILE= ARCH=arm64 -j5 W=4 > > M=drivers/crypto/keembay > > Not sure what would happen with 'W=4'. Sorry that was a typo (I meant to write W=1) :/ > > Probably nothing, as it only goes up to 3 [0]. > > > Which command are you running exactly? I'll use it for my next > > submissions. > > rm -rf ../builds/build-arm64/drivers/crypto/ > make -f Makefile -j24 --quiet ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- > KBUILD_OUTPUT=../builds/build-arm64 allmodconfig > make -f Makefile -j24 --quiet ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- > KBUILD_OUTPUT=../builds/build-arm64 W=1 drivers/crypto/ > > Hope that helps. Thanks for providing your commands. Unfortunately, even if I run them I don't get the above warnings. I wonder if the issue is in my version of scripts/kernel-doc (which seems to be the script called by the Makefile to to check the kernel- doc). I'll keep investigating this. Thanks again! > > [0] scripts/Makefile.extrawarn >
Re: [PATCH 06/20] crypto: keembay: ocs-hcu: Fix incorrectly named functions/structs
On Thu, 2021-02-04 at 11:09 +, Lee Jones wrote: > Fixes the following W=1 kernel build warning(s): > > drivers/crypto/keembay/ocs-hcu.c:107: warning: expecting prototype for > struct ocs_hcu_dma_list. Prototype was for struct ocs_hcu_dma_entry instead > drivers/crypto/keembay/ocs-hcu.c:127: warning: expecting prototype for > struct ocs_dma_list. Prototype was for struct ocs_hcu_dma_list instead > drivers/crypto/keembay/ocs-hcu.c:610: warning: expecting prototype for > ocs_hcu_digest(). Prototype was for ocs_hcu_hash_update() instead > drivers/crypto/keembay/ocs-hcu.c:648: warning: expecting prototype for > ocs_hcu_hash_final(). Prototype was for ocs_hcu_hash_finup() instead > > Cc: Daniele Alessandrelli > Cc: Declan Murphy > Cc: Herbert Xu > Cc: "David S. Miller" > Cc: linux-cry...@vger.kernel.org > Signed-off-by: Lee Jones > --- Acked-by: Daniele Alessandrelli Thanks for fixing these. For some reason, if the issues are there, I don't get those warnings when compiling with W=1; the command I run is: make CROSS_COMPILE= ARCH=arm64 -j5 W=4 M=drivers/crypto/keembay Which command are you running exactly? I'll use it for my next submissions.
Re: [PATCH v7 4/7] crypto: add ecc curve and expose them
On Tue, 2021-02-02 at 20:42 +1100, Herbert Xu wrote: > On Tue, Feb 02, 2021 at 09:27:33AM +0000, Alessandrelli, Daniele > wrote: > > I see. Just to clarify: does the in-kernel user requirement also > > apply > > to the case when the author of a device driver also provides the > > software implementation for the new algorithms supported by device > > driver / HW? > > Yes we need an actual user. For example, if your algorithm is used > by the Security Subsystem (IMA) that would be sufficient. Understood, thanks! Unrelated question: I have my Keem Bay OCS ECC patchset [1] almost ready for re-submission. Should I go ahead or should I wait for the final decision about using 'ecdh-nist-pXXX' in place of 'ecdh'? Also, I guess I'll have to drop P-384 support from the driver, since the are no in-kernel user AFAIK. [1] https://lore.kernel.org/linux-crypto/20201217172101.381772-1-daniele.alessandre...@linux.intel.com/ > > Cheers,
Re: [PATCH v7 4/7] crypto: add ecc curve and expose them
On Tue, 2021-02-02 at 16:13 +1100, Herbert Xu wrote: > The issue is that we always require a software implementation for > any given hardware algorithm. As otherwise kernel users cannot > rely on the algorithm to work. I understand. This sounds very reasonable to me. > Of course we don't want to add every single algorithm out there > to the kernel so that's why require there to be an actual in-kernel > user before adding a given algorithm. I see. Just to clarify: does the in-kernel user requirement also apply to the case when the author of a device driver also provides the software implementation for the new algorithms supported by device driver / HW?
Re: [PATCH v3 03/34] mailbox: vpu-ipc-mailbox: Add support for Intel VPU IPC mailbox
Hi Jassi,
Thank you very much for your feedback.
On Mon, 2021-02-01 at 01:07 -0600, Jassi Brar wrote:
> On Fri, Jan 29, 2021 at 8:21 PM wrote:
> > From: Daniele Alessandrelli
> >
> > Add mailbox controller enabling inter-processor communication (IPC)
> > between the CPU (aka, the Application Processor - AP) and the VPU on
> > Intel Movidius SoCs like Keem Bay.
> >
> > The controller uses HW FIFOs to enable such communication. Specifically,
> > there are two FIFOs, one for the CPU and one for VPU. Each FIFO can hold
> > 128 entries (messages) of 32-bit each (but only 26 bits are actually
> > usable, since the 6 least-significant bits are reserved).
> >
> > When the Linux kernel on the AP needs to send messages to the VPU
> > firmware, it writes them to the VPU FIFO; similarly, when the VPU
> > firmware needs to send messages to the AP, it writes them to the CPU
> > FIFO.
> >
> > The AP is notified of pending messages in the CPU FIFO by means of the
> > 'FIFO-not-empty' interrupt, which is generated by the CPU FIFO while not
> > empty. This interrupt is cleared automatically once all messages have
> > been read from the FIFO (i.e., the FIFO has been emptied).
> >
> > The hardware doesn't provide an TX done IRQ (i.e., an IRQ that allows
> > the VPU firmware to notify the AP that the message put into the VPU FIFO
> > has been received); however the AP can ensure that the message has been
> > successfully put into the VPU FIFO (and therefore transmitted) by
> > checking the VPU FIFO status register to ensure that writing the message
> > didn't cause the FIFO to overflow.
> >
> > Therefore, the mailbox controller is configured as capable of tx_done
> > IRQs and a tasklet is used to simulate the tx_done IRQ. The tasklet is
> > activated by send_data() right after the message has been put into the
> > VPU FIFO and the VPU FIFO status registers has been checked. If an
> > overflow is reported by the status register, the tasklet passes -EBUSY
> > to mbox_chan_txdone(), to notify the mailbox client of the failed TX.
> >
> > The client should therefore register a tx_done() callback to properly
> > handle failed transmissions.
> >
> > Note: the 'txdone_poll' mechanism cannot be used because it doesn't
> > provide a way to report a failed transmission.
> >
> txdone means the last submitted transfer has been done with --
> successfully or not.
Yes, that's usually the case, but not for poll mode (at least from what
I can tell).
last_tx_done() can return only true or false, but when true is
returned, there is no way to know if the TX was successful or not; the
mailbox framework just seems to assume that 'true' means "message
successfully transmitted", since 0 is passed to tx_tick() (end
eventually to the tx_done() client callback):
https://github.com/torvalds/linux/blob/master/drivers/mailbox/mailbox.c#L131
If 'false' is returned, the polling continues until the timeout is reached.
(please correct me if my above understanding is wrong)
> So I think we can do without the tasklet as explained below
>
>
>
> > +static int vpu_ipc_mailbox_send_data(struct mbox_chan *chan, void *data)
> > +{
> > + struct vpu_ipc_mbox *vpu_ipc_mbox = chan->con_priv;
> > + u32 entry, overflow;
> > +
> > + entry = *((u32 *)data);
> > +
> Are all messages max 32bits wide?
> Usually the controller specifies a packet format (more than just a
> word but of course that's not mandatory) that a client submits the
> data to be transmitted in. Esp when it has deep FIFOs.
It's actually only 26 bits, since the last 6 bits are reserved for the
overflow detection mechanism; I should probably have explained this
better in the commit message, sorry!
Basically, the AP is not the only one that can write to the VPU FIFO:
other components within the SoC can write to it too. Each of these
components has a unique 6-bit processor ID associated to it. The HW
FIFO expects that the last 6 bits of each 32-bit FIFO entry contain the
processor ID of the sender.
Sending a message works as follows:
1. The message must be a 32-bit value with the last 6-bit set to 0 (in
practice, the message is meant to be a 32-bit address value, aligned
to 64 bytes).
2. The sender adds its processor ID to the 32-bit message being sent: M
= m | ProcID
3. The sender writes the message (M) to the TIM_IPC_FIFO register
4. The HW atomically checks if the FIFO is full and if not it writes it
to the actual FIFO; if the FIFO is full, the HW reads the ProcID
from M and then sets the corresponding bit of TIM_IPC_FIFO_OF_FLAG0,
to signal that the write failed, because the FIFO was full
5. The sender reads the TIM_IPC_FIFO_OF_FLAG0 register and checks if
the bit corresponding to its ProcID has been set (in order to know
if the TX succeeded or failed); if the bit is set, the sender clears
it.
Note: as briefly mentioned above, the 32-bit value is meant to be a 32-
bit physical address
Re: [PATCH v2 06/34] dt-bindings: Add bindings for Keem Bay VPU IPC driver
Hi Rob, Thanks for your review. On Mon, 2021-01-11 at 13:24 -0600, Rob Herring wrote: > On Fri, Jan 08, 2021 at 01:25:32PM -0800, mgr...@linux.intel.com > wrote: > > From: Paul Murphy > > > > Add DT bindings documentation for the Keem Bay VPU IPC driver. > > > > Cc: Rob Herring > > Cc: devicet...@vger.kernel.org > > Reviewed-by: Mark Gross > > Signed-off-by: Paul Murphy > > Co-developed-by: Daniele Alessandrelli < > > daniele.alessandre...@intel.com> > > Signed-off-by: Daniele Alessandrelli < > > daniele.alessandre...@intel.com> > > Needs your Sob. > > > --- > > .../soc/intel/intel,keembay-vpu-ipc.yaml | 153 > > ++ > > This doesn't fit somewhere else? It's quite a SoC-specific driver, designed to control (start, stop, monitor, etc) the Vision Processing Unit (VPU) integrated in the Keem Bay SoC. Do you think it would fit better somewhere else? > > > 1 file changed, 153 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu- > > ipc.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu- > > ipc.yaml > > b/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu- > > ipc.yaml > > new file mode 100644 > > index ..cd1c4abe8bc9 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/soc/intel/intel,keembay- > > vpu-ipc.yaml > > @@ -0,0 +1,153 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +# Copyright (c) Intel Corporation. All rights reserved. > > +%YAML 1.2 > > +--- > > +$id: " > > http://devicetree.org/schemas/soc/intel/intel,keembay-vpu-ipc.yaml# > > " > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#; > > + > > +title: Intel Keem Bay VPU IPC > > + > > +maintainers: > > + - Paul Murphy > > + > > +description: > > + The VPU IPC driver facilitates loading of firmware, control, and > > communication > > + with the VPU over the IPC FIFO in the Intel Keem Bay SoC. > > VPU is never defined. We'll spell out the acronym in v3. Anyway, VPU = Vision Processing Unit > > Bindings are for h/w blocks, not drivers. Will be fixed in v3 > > > + > > +properties: > > + compatible: > > +oneOf: > > + - items: > > +- const: intel,keembay-vpu-ipc > > + > > + reg: > > +items: > > + - description: NCE WDT registers > > + - description: NCE TIM_GEN_CONFIG registers > > + - description: MSS WDT registers > > + - description: MSS TIM_GEN_CONFIG registers > > + > > + reg-names: > > +items: > > + - const: nce_wdt > > + - const: nce_tim_cfg > > + - const: mss_wdt > > + - const: mss_tim_cfg > > + > > + memory-region: > > +items: > > + - description: reference to the VPU reserved memory region > > + - description: reference to the X509 reserved memory region > > + - description: reference to the MSS IPC area > > + > > + clocks: > > +items: > > + - description: cpu clock > > + - description: pll 0 out 0 rate > > + - description: pll 0 out 1 rate > > + - description: pll 0 out 2 rate > > + - description: pll 0 out 3 rate > > + - description: pll 1 out 0 rate > > + - description: pll 1 out 1 rate > > + - description: pll 1 out 2 rate > > + - description: pll 1 out 3 rate > > + - description: pll 2 out 0 rate > > + - description: pll 2 out 1 rate > > + - description: pll 2 out 2 rate > > + - description: pll 2 out 3 rate > > + > > + clock-names: > > +items: > > + - const: cpu_clock > > + - const: pll_0_out_0 > > + - const: pll_0_out_1 > > + - const: pll_0_out_2 > > + - const: pll_0_out_3 > > + - const: pll_1_out_0 > > + - const: pll_1_out_1 > > + - const: pll_1_out_2 > > + - const: pll_1_out_3 > > + - const: pll_2_out_0 > > + - const: pll_2_out_1 > > + - const: pll_2_out_2 > > + - const: pll_2_out_3 > > + > > + interrupts: > > +items: > > + - description: number of NCE sub-system WDT timeout IRQ > > + - description: number of MSS sub-system WDT timeout IRQ > > + > > + interrupt-names: > > +items: > > + - const: nce_wdt > > + - const: mss_wdt > > + > > + intel,keembay-vpu-ipc-nce-wdt-redirect: > > +$ref: "/schemas/types.yaml#/definitions/uint32" > > +description: > > + Number to which we will request that the NCE sub-system > > + re-directs it's WDT timeout IRQ > > + > > + intel,keembay-vpu-ipc-mss-wdt-redirect: > > +$ref: "/schemas/types.yaml#/definitions/uint32" > > +description: > > + Number to which we will request that the MSS sub-system > > + re-directs it's WDT timeout IRQ > > These look like the same value as the interrupt numbers? That's a very good point. We'll drop these additional properties and re-use the interrupt numbers. > > > + > > + intel,keembay-vpu-ipc-imr: > > +$ref: "/schemas/types.yaml#/definitions/uint32" > > +description: > > +
Re: [PATCH v5 3/5] crypto: expose elliptic curve parameters as Crypto APIs
On Mon, 2021-01-04 at 17:36 +0800, yumeng wrote: > > 在 2021/1/3 5:29, Herbert Xu 写道: > > On Thu, Dec 24, 2020 at 02:08:25PM +0800, Meng Yu wrote: > > > Move elliptic curves definition to 'include/crypto/ecc_curve_defs.h', > > > so all can use it, > > > > > > Signed-off-by: Meng Yu > > > Reviewed-by: Zaibo Xu > > > --- > > > crypto/ecc.c| 1 - > > > crypto/ecc.h| 37 + > > > crypto/ecc_curve_defs.h | 57 - > > > crypto/ecrdsa_defs.h| 2 +- > > > include/crypto/ecc_curve_defs.h | 92 > > > + > > > 5 files changed, 95 insertions(+), 94 deletions(-) > > > delete mode 100644 crypto/ecc_curve_defs.h > > > create mode 100644 include/crypto/ecc_curve_defs.h > > > > This conflicts with > > > > https://patchwork.kernel.org/project/linux-crypto/patch/20201217172101.381772-3-daniele.alessandre...@linux.intel.com/ > > > > Please discuss with each other on how you would like to proceed. > > > > Thanks, > > > > hello, Daniele, > > In my patch, I move elliptic curves definition to > 'include/crypto/ecc_curve_defs.h', > which include the P-384 curve you need, and you can easily import it to > your driver. > > And if you include 'crypto/ecc_curve_defs.h', > 'drivers/crypto/keembay/ocs-ecc-curve-defs.h' > is not needed. > > Could you think about it, to rely on my patchset? Hi Meng, Sure, I can update my RFC patch series to rely on your patchset. It makes sense to do so, since, as you said, it will allow me to get rid of my ocs-ecc-curve-defs.h (and also because your patchset will probably be merged before mine). Herbert, should I send an updated RFC now or should I wait for the discussion on the RFC open points to be completed first? Thanks, Daniele
Re: keembay-ocs-aes-core.c:undefined reference to `devm_ioremap_resource'
On Thu, 2020-12-17 at 13:23 +0800, kernel test robot wrote: > tree: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > master > head: accefff5b547a9a1d959c7e76ad539bf2480e78b > commit: 88574332451380f4b51f6ca88ab9810e714bfb9b crypto: keembay - > Add support for Keem Bay OCS AES/SM4 > date: 6 days ago > config: s390-randconfig-c004-20201217 (attached as .config) > compiler: s390-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 > # > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=88574332451380f4b51f6ca88ab9810e714bfb9b > git remote add linus > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > git fetch --no-tags linus master > git checkout 88574332451380f4b51f6ca88ab9810e714bfb9b > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 > make.cross ARCH=s390 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All errors (new ones prefixed by >>): > >s390-linux-ld: kernel/dma/coherent.o: in function > `dma_init_coherent_memory': >coherent.c:(.text+0x54c): undefined reference to `memremap' >s390-linux-ld: coherent.c:(.text+0x67e): undefined reference to > `memunmap' >s390-linux-ld: kernel/dma/coherent.o: in function > `dma_declare_coherent_memory': >coherent.c:(.text+0x8ee): undefined reference to `memunmap' >s390-linux-ld: drivers/clk/clk-fixed-mmio.o: in function > `fixed_mmio_clk_setup': >clk-fixed-mmio.c:(.text+0x98): undefined reference to `of_iomap' >s390-linux-ld: clk-fixed-mmio.c:(.text+0xe8): undefined reference > to `iounmap' >s390-linux-ld: drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.o: in > function `dw_probe': >dw-axi-dmac-platform.c:(.text+0x20ac): undefined reference to > `devm_ioremap_resource' >s390-linux-ld: drivers/dma/fsl-edma.o: in function > `fsl_edma_probe': >fsl-edma.c:(.text+0x1238): undefined reference to > `devm_ioremap_resource' >s390-linux-ld: fsl-edma.c:(.text+0x1628): undefined reference to > `devm_ioremap_resource' >s390-linux-ld: drivers/dma/sf-pdma/sf-pdma.o: in function > `sf_pdma_probe': >sf-pdma.c:(.text+0x92e): undefined reference to > `devm_ioremap_resource' >s390-linux-ld: drivers/crypto/ccree/cc_driver.o: in function > `init_cc_resources': >cc_driver.c:(.text+0x89e): undefined reference to > `devm_ioremap_resource' >s390-linux-ld: drivers/crypto/ccree/cc_debugfs.o: in function > `cc_debugfs_init': >cc_debugfs.c:(.text+0x108): undefined reference to > `debugfs_create_regset32' >s390-linux-ld: cc_debugfs.c:(.text+0x206): undefined reference to > `debugfs_create_regset32' >s390-linux-ld: drivers/crypto/keembay/keembay-ocs-aes-core.o: in > function `kmb_ocs_aes_probe': > > > keembay-ocs-aes-core.c:(.text+0x1ed2): undefined reference to > > > `devm_ioremap_resource' Looks like this is due to a missing HAS_IOMEM dependency. Herbert, I'll prepare a patch fixing this and I'll send it to you. > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH] crypto: CRYPTO_DEV_KEEMBAY_OCS_AES_SM4 should depend on ARCH_KEEMBAY
Thanks for the patch.
On Wed, 2020-12-16 at 14:14 +0100, Geert Uytterhoeven wrote:
> The Intel Keem Bay Offload and Crypto Subsystem (OCS) is only present
> on
> Intel Keem Bay SoCs. Hence add a dependency on ARCH_KEEMBAY, to
> prevent
> asking the user about this driver when configuring a kernel without
> Intel Keem Bay platform support.
>
> While at it, fix a misspelling of "cipher".
>
> Fixes: 88574332451380f4 ("crypto: keembay - Add support for Keem Bay
> OCS AES/SM4")
> Signed-off-by: Geert Uytterhoeven
Acked-by: Daniele Alessandrelli
> ---
> drivers/crypto/keembay/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/keembay/Kconfig
> b/drivers/crypto/keembay/Kconfig
> index 3c16797b25b9497d..6f62c838a3fa0b2e 100644
> --- a/drivers/crypto/keembay/Kconfig
> +++ b/drivers/crypto/keembay/Kconfig
> @@ -1,12 +1,12 @@
> config CRYPTO_DEV_KEEMBAY_OCS_AES_SM4
> tristate "Support for Intel Keem Bay OCS AES/SM4 HW
> acceleration"
> - depends on OF || COMPILE_TEST
> + depends on ARCH_KEEMBAY || COMPILE_TEST
> select CRYPTO_SKCIPHER
> select CRYPTO_AEAD
> select CRYPTO_ENGINE
> help
> Support for Intel Keem Bay Offload and Crypto Subsystem (OCS)
> AES and
> - SM4 cihper hardware acceleration for use with Crypto API.
> + SM4 cipher hardware acceleration for use with Crypto API.
>
> Provides HW acceleration for the following transformations:
> cbc(aes), ctr(aes), ccm(aes), gcm(aes), cbc(sm4), ctr(sm4),
> ccm(sm4)
Re: [PATCH 1/1] soc: keembay: Add Keem Bay IMR driver
Hi Arnd, Pavel, Thanks for your feedback. > > > > > > The problem is that I need this code to be run early at boot, so > > > I > > > don't think I can make this a module. > > > > How early is early enough? Basically, before any device with direct memory access is activated (because if anybody, except the ARM CPU, tries to access that memory, the memory protection mechanism will be triggered and a reboot will happen). > > > > What bootloader are you using? U-Boot > > > > I believe you should simply fix your bootloader not to pass locked > > memory to the kernel. The bootloader is behaving like that for security reasons, so we'd like to avoid modifying it. I'll provide more information below. > > > > Alternatively, take that memory off the "memory available" maps, > > and only re-add it once > > it is usable. > > > > Anything else is dangerous. That sounds like an interesting solution, thanks! Do you know any code that I can use as a reference? Since, the protected memory is just a few megabytes large, I guess that in theory we could just have U-Boot mark it as reserved memory and forget about it; but if there's a way to re-add it back once in Linux that would be great. > > Agreed, this sounds like an incompatible extension of the boot > protocol > that we should otherwise not merge. > > However, there is also a lot of missing information here, and it is > always > possible they are trying to something for a good reason. As long as > the > problem that the bootloader is trying to solve is explained well > enough > in the changelog, we can discuss it to see how it should be done > properly. Apologies, I should have provided more information. Here it is :) Basically, at boot time U-Boot code and core memory (.text, .data, .bss, etc.) is protected by this Isolated Memory Region (IMR) which prevents any device or processing units other than the ARM CPU to access/modify the memory. This is done for security reasons, to reduce the risks that a potential attacker can use "hijacked" HW devices to interfere with the boot process (and break the secure boot flow in place). Before booting the Kernel, U-Boot sets up a new IMR to protect the Kernel image (so that the kernel can benefit of a similar protection) and then starts the kernel, delegating to the kernel the task of switching off the U-Boot IMR. U-Boot doesn't turn off its own IMR because doing so would leave a (tiny) window in which the boot execution flow is not protected. If you have any additional questions or feedback, just let me know. Regards, Daniele -- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
Re: [PATCH 0/1] Add IMR driver for Keem Bay
On Tue, 2020-04-21 at 17:36 +0100, Daniele Alessandrelli wrote: > The following is a patch for a new Intel Movidius SoC, code-named > Keem > Bay. > > Keem Bay needs a driver to disable the Isolated Memory Region (IMR) > set up by the SoC bootloader during early boot. > > If such an IMR is not disabled and some device tries to access it, > the system will reboot. > > Since this driver is SoC-specific and Keem Bay is a new SoC, I was > unsure of where to put this driver. In the end I decided to create a > new 'keembay' directory in 'drivers/soc'. I hope that's reasonable, > if > not, just let me know. > > > Daniele Alessandrelli (1): > soc: keembay: Add Keem Bay IMR driver > > MAINTAINERS | 5 > drivers/soc/Kconfig | 1 + > drivers/soc/Makefile | 1 + > drivers/soc/keembay/Kconfig | 22 + > drivers/soc/keembay/Makefile | 5 > drivers/soc/keembay/keembay-imr.c | 40 > +++ > 6 files changed, 74 insertions(+) > create mode 100644 drivers/soc/keembay/Kconfig > create mode 100644 drivers/soc/keembay/Makefile > create mode 100644 drivers/soc/keembay/keembay-imr.c > Adding some more people (Arnd and linux-arm-kernel ML) in CC in the hope of getting some guidance on how to have this patch merged. I'm a novice, so I wonder if the lack of feedback is because I'm doing something wrong or if I just sent the initial email to the wrong people / ML. I'd appreciate any help you can provide. -- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
