Re: [PATCH v11 06/13] crypto: aesni: add minimal build option for SGX LE

[Sean Christopherson]

On Fri, 2018-06-08 at 10:27 -0700, Dave Hansen wrote:
> On 06/08/2018 10:09 AM, Jarkko Sakkinen wrote:
> > 
> > --- a/arch/x86/crypto/aesni-intel_asm.S
> > +++ b/arch/x86/crypto/aesni-intel_asm.S
> > @@ -45,6 +45,8 @@
> >  #define MOVADQ movaps
> >  #define MOVUDQ movups
> >  
> > +#ifndef AESNI_INTEL_MINIMAL
> > +
> >  #ifdef __x86_64__
> >  
> >  # constants in mergeable sections, linker can reorder and merge
> > @@ -133,6 +135,8 @@ ALL_F:  .octa 0x
> >  #define keysize 2*15*16(%arg1)
> >  #endif
> >  
> > +#endif /* AESNI_INTEL_MINIMAL */
> > +
> I'd really prefer that these get moved into a separate file rather than
> a scattered set of #ifdefs.  This just seem fragile to me.
> 
> Can we have a "aesni-intel_asm-minimal.S"?  Or, at least bunch the
> minimal set of things *together*?

A separate file doesn't seem appropriate because there is no criteria
for including code in the "minimal" build beyond "this code happens to
be needed by SGX".  I considered having SGX somewhere in the define
but opted for AESNI_INTEL_MINIMAL on the off chance that the minimal
build was useful for something other than SGX.

I'm not opposed to bunching the minimal stuff together, my intent was
simply to disturb the code as little as possible.

Re: [PATCH 2/9] crypto: atmel-ecc: Silently ignore missing clock frequency

[Tudor Ambarus]

Hi, Linus, Wolfram,

On 06/05/2018 04:49 PM, Linus Walleij wrote:

The Atmel ECC driver contains a check for the I2C bus clock
frequency, so as to check that the I2C adapter in use
satisfies the device specs.

If the device is connected to a device tree node that does not
contain a clock frequency setting, such as an I2C mux or gate,
this blocks the probe. Make the probe continue silently if
no clock frequency can be found, assuming all is safe.


I don't think it's safe. We use bus_clk_rate to compute the ecc508's
wake token. If you can't wake the device, it will ignore all your
commands.

My proposal was to introduce a bus_freq_hz member in the i2c_adapter
structure (https://patchwork.kernel.org/patch/10307637/), but I haven't
received feedback yet, Wolfram?

I would say first to check the bus_freq_hz from adapter (if it will be
accepted) else to look into dt and, in the last case, if nowhere
provided, to block the probe.

Thanks,
ta



Signed-off-by: Linus Walleij 
---
  drivers/crypto/atmel-ecc.c | 11 +--
  1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index e66f18a0ddd0..145ab3a39a56 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -657,14 +657,13 @@ static int atmel_ecc_probe(struct i2c_client *client,
return -ENODEV;
}
  
+	/*

+* Silently assume all is fine if there is no
+* "clock-frequency" property.
+*/
ret = of_property_read_u32(client->adapter->dev.of_node,
   "clock-frequency", _clk_rate);
-   if (ret) {
-   dev_err(dev, "of: failed to read clock-frequency property\n");
-   return ret;
-   }
-
-   if (bus_clk_rate > 100L) {
+   if (!ret && (bus_clk_rate > 100L)) {
dev_err(dev, "%d exceeds maximum supported clock frequency 
(1MHz)\n",
bus_clk_rate);
return -EINVAL;