Re: [RFC PATCH v2 07/20] x86: Provide general kernel support for memory encryption

2016-09-08 Thread Tom Lendacky 
On 09/07/2016 10:55 AM, Borislav Petkov wrote:
> On Wed, Sep 07, 2016 at 09:30:54AM -0500, Tom Lendacky wrote:
>> _PAGE_ENC is #defined as sme_me_mask and sme_me_mask has already been
>> set (or not set) at this point - so it will be the mask if SME is
>> active or 0 if SME is not active.
> 
> Yeah, I remember :-)
> 
>> sme_early_init() is merely propagating the mask to other structures.
>> Since early_pmd_flags is mainly used in this file (one line in
>> head_64.S is the other place) I felt it best to modify it here. But it
>> can always be moved if you feel that is best.
> 
> Hmm, so would it work then if you stick it in early_pmd_flags'
> definition like you do with the other masks? I.e.,
> 
> pmdval_t early_pmd_flags = __PAGE_KERNEL_LARGE | _PAGE_ENC & ~(_PAGE_GLOBAL | 
> _PAGE_NX);

When does this value get initialized?  Since _PAGE_ENC is #defined to
sme_me_mask, which is not set until the boot process begins, I'm afraid
we'd end up using the initial value of sme_me_mask, which is zero.  Do
I have that right?

Thanks,
Tom

> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH v2 07/20] x86: Provide general kernel support for memory encryption

2016-09-08 Thread Borislav Petkov 
On Thu, Sep 08, 2016 at 08:26:27AM -0500, Tom Lendacky wrote:
> When does this value get initialized?  Since _PAGE_ENC is #defined to
> sme_me_mask, which is not set until the boot process begins, I'm afraid
> we'd end up using the initial value of sme_me_mask, which is zero.  Do
> I have that right?

Hmm, but then that would hold true for all the other defines where you
OR-in _PAGE_ENC, no?

In any case, the preprocessed source looks like this:

pmdval_t early_pmd_flags = (((pteval_t)(1)) << 0) | (((pteval_t)(1)) << 1) 
| (((pteval_t)(1)) << 6) | (((pteval_t)(1)) << 5) | (((pteval_t)(1)) << 8)) | 
(((pteval_t)(1)) << 63)) | (((pteval_t)(1)) << 7)) | sme_me_mask) & 
~pteval_t)(1)) << 8) | (((pteval_t)(1)) << 63));

but the problem is later, when building:

arch/x86/kernel/head64.c:39:28: error: initializer element is not constant
 pmdval_t early_pmd_flags = (__PAGE_KERNEL_LARGE | _PAGE_ENC) & ~(_PAGE_GLOBAL 
| _PAGE_NX);
^
scripts/Makefile.build:153: recipe for target 'arch/x86/kernel/head64.s' failed

so I guess not. :-\

Ok, then at least please put the early_pmd_flags init after
sme_early_init() along with a small comment explaning what happens.

Thanks.

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: Stability connection problems in ath9k kernel 4.7

2016-09-08 Thread Valerio Passini 
I'm hoping having done it right and I can try your first suggestion, but I
really cannot solve this problem by myself: sorry, I have no capabilities
in programming in any known and unknown computer language. Surely, I can
test all the patches you want and report the results but this is the best I
can do. Best regards

Valerio

Il 08/Set/2016 19:25, "Kalle Valo"  ha scritto:

> Valerio Passini  writes:
>
> > On mercoledì 7 settembre 2016 11:32:24 CEST Kalle Valo wrote:
> >> Valerio Passini  writes:
> >> > I have found some connection problems since 4.7 release using ath9k
> that
> >> > turn the wifi pretty useless, I think it might be something in the
> power
> >> > management because the signal seems really low. Previously, up to
> kernel
> >> > 4.6.7 everything worked very well.
> >> >
> >> > This is a sample of dmesg in kernel 4.7.2:
> >> >  239.898935] wlp4s0: authenticate with XX:XX:XX:XX:XX:XX
> >> >
> >> > [  239.919995] wlp4s0: send auth to XX:XX:XX:XX:XX:XX  (try 1/3)
> >> > [  239.931877] wlp4s0: authenticated
> >> > [  239.932357] wlp4s0: associate with XX:XX:XX:XX:XX:XX  (try 1/3)
> >> > [  239.942171] wlp4s0: RX AssocResp from XX:XX:XX:XX:XX:XX
> (capab=0x431
> >> > status=0 aid=2)
> >> > [  239.942301] wlp4s0: associated
> >> > [  244.802853] ath: phy0: DMA failed to stop in 10 ms AR_CR=0x0024
> >> > AR_DIAG_SW=0x0220 DMADBG_7=0x
> >> > 6100
> >> > [  245.931832] wlp4s0: authenticate with XX:XX:XX:XX:XX:XX
> >> > [  245.953028] wlp4s0: send auth to XX:XX:XX:XX:XX:XX  (try 1/3)
> >> > [  245.958702] wlp4s0: authenticated
> >> > [  245.960386] wlp4s0: associate withXX:XX:XX:XX:XX:XX  (try 1/3)
> >> > [  245.980543] wlp4s0: RX AssocResp from XX:XX:XX:XX:XX:XX
> (capab=0x431
> >> > status=0 aid=2)
> >> >
> >> > lspci on 4.6.7 kernel:
> >> > 04:00.0 Network controller: Qualcomm Atheros AR9485 Wireless Network
> >> > Adapter (rev 01)
> >> >
> >> > Subsystem: AzureWave AR9485 Wireless Network Adapter
> >> > Flags: bus master, fast devsel, latency 0, IRQ 18
> >> > Memory at f790 (64-bit, non-prefetchable) [size=512K]
> >> > Expansion ROM at f798 [disabled] [size=64K]
> >> > Capabilities: [40] Power Management version 2
> >> > Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+
> >> > Capabilities: [70] Express Endpoint, MSI 00
> >> > Capabilities: [100] Advanced Error Reporting
> >> > Capabilities: [140] Virtual Channel
> >> > Capabilities: [160] Device Serial Number
> 00-00-00-00-00-00-00-00
> >> > Kernel driver in use: ath9k
> >> > Kernel modules: ath9k
> >> >
> >> > Probably you need some debugging output, but before recompiling the
> kernel
> >> > I would like to know if you are interested in any kind of help from me
> >> > and what steps I should take (I'm able to help in testing patches but
> I'm
> >> > not familiar with git). Thank you
> >>
> >> Usually it's really helpful if you can find the commit id which broke
> >> it. 'git bisect' is a great tool to do that and this seems to be a nice
> >> tutorial how to use it:
> >>
> >> http://webchick.net/node/99
> >>
> >> Instead of commit ids you can use release tags like v4.6 and v4.7 to
> >> make it easier to start the bisect. Just make sure that v4.7 is really
> >> broken and v4.6 works before you start the bisection.
> >
> > Hi Kalle,
> >
> > I tried to understand the whole procedure related to git and git bisect,
> and
> > this is the first time I try it, so I can have done some mistake. In the
> git
> > log you'll find the commit that could be guilty for the behaviour I
> reported
> > yesterday. Anyhow, the resulting commit doesn't make any sense to me.
>
> So your bisect found this as the bad commit:
>
> commit 9257b4a206fc0229dd5f84b78e4d1ebf3f91d270
> Author: Omer Peleg 
> Date:   Wed Apr 20 11:34:11 2016 +0300
>
> iommu/iova: introduce per-cpu caching to iova allocation
>
> The ath9k log you provided has a DMA warning and iommu problems can
> cause DMA problems but I cannot make any conclusions yet. To confirm
> that this commit really is the problem you could try to revert it with
> 'git revert -n 9257b4a206fc0229dd5f84b78e4d1ebf3f91d270'. For some
> reason I got conflicts but if you are good enough with C you could try
> to fix those yourself. Another option is that you disable iommu and see
> if that helps.
>
> I'm adding more people and mailing lists related to this commit,
> hopefully they have better ideas.
>
> This is Valerio's bisect log:
>
> git bisect start
> # good: [2dcd0af568b0cf583645c8a317dd12e344b1c72a] Linux 4.6
> git bisect good 2dcd0af568b0cf583645c8a317dd12e344b1c72a
> # bad: [523d939ef98fd712632d93a5a2b588e477a7565e] Linux 4.7
> git bisect bad 523d939ef98fd712632d93a5a2b588e477a7565e
> # good: [0694f0c9e20c47063e4237e5f6649ae5ce5a369a] radix tree test suite:
> remove dependencies on height
> git

Re: Stability connection problems in ath9k kernel 4.7

2016-09-08 Thread Kalle Valo 
Valerio Passini  writes:

> On mercoledì 7 settembre 2016 11:32:24 CEST Kalle Valo wrote:
>> Valerio Passini  writes:
>> > I have found some connection problems since 4.7 release using ath9k that
>> > turn the wifi pretty useless, I think it might be something in the power
>> > management because the signal seems really low. Previously, up to kernel
>> > 4.6.7 everything worked very well.
>> > 
>> > This is a sample of dmesg in kernel 4.7.2:
>> >  239.898935] wlp4s0: authenticate with XX:XX:XX:XX:XX:XX
>> > 
>> > [  239.919995] wlp4s0: send auth to XX:XX:XX:XX:XX:XX  (try 1/3)
>> > [  239.931877] wlp4s0: authenticated
>> > [  239.932357] wlp4s0: associate with XX:XX:XX:XX:XX:XX  (try 1/3)
>> > [  239.942171] wlp4s0: RX AssocResp from XX:XX:XX:XX:XX:XX  (capab=0x431
>> > status=0 aid=2)
>> > [  239.942301] wlp4s0: associated
>> > [  244.802853] ath: phy0: DMA failed to stop in 10 ms AR_CR=0x0024
>> > AR_DIAG_SW=0x0220 DMADBG_7=0x
>> > 6100
>> > [  245.931832] wlp4s0: authenticate with XX:XX:XX:XX:XX:XX
>> > [  245.953028] wlp4s0: send auth to XX:XX:XX:XX:XX:XX  (try 1/3)
>> > [  245.958702] wlp4s0: authenticated
>> > [  245.960386] wlp4s0: associate withXX:XX:XX:XX:XX:XX  (try 1/3)
>> > [  245.980543] wlp4s0: RX AssocResp from XX:XX:XX:XX:XX:XX  (capab=0x431
>> > status=0 aid=2)
>> > 
>> > lspci on 4.6.7 kernel:
>> > 04:00.0 Network controller: Qualcomm Atheros AR9485 Wireless Network
>> > Adapter (rev 01)
>> > 
>> > Subsystem: AzureWave AR9485 Wireless Network Adapter
>> > Flags: bus master, fast devsel, latency 0, IRQ 18
>> > Memory at f790 (64-bit, non-prefetchable) [size=512K]
>> > Expansion ROM at f798 [disabled] [size=64K]
>> > Capabilities: [40] Power Management version 2
>> > Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+
>> > Capabilities: [70] Express Endpoint, MSI 00
>> > Capabilities: [100] Advanced Error Reporting
>> > Capabilities: [140] Virtual Channel
>> > Capabilities: [160] Device Serial Number 00-00-00-00-00-00-00-00
>> > Kernel driver in use: ath9k
>> > Kernel modules: ath9k
>> > 
>> > Probably you need some debugging output, but before recompiling the kernel
>> > I would like to know if you are interested in any kind of help from me
>> > and what steps I should take (I'm able to help in testing patches but I'm
>> > not familiar with git). Thank you
>> 
>> Usually it's really helpful if you can find the commit id which broke
>> it. 'git bisect' is a great tool to do that and this seems to be a nice
>> tutorial how to use it:
>> 
>> http://webchick.net/node/99
>> 
>> Instead of commit ids you can use release tags like v4.6 and v4.7 to
>> make it easier to start the bisect. Just make sure that v4.7 is really
>> broken and v4.6 works before you start the bisection.
>
> Hi Kalle,
>
> I tried to understand the whole procedure related to git and git bisect, and 
> this is the first time I try it, so I can have done some mistake. In the git 
> log you'll find the commit that could be guilty for the behaviour I reported 
> yesterday. Anyhow, the resulting commit doesn't make any sense to me.

So your bisect found this as the bad commit:

commit 9257b4a206fc0229dd5f84b78e4d1ebf3f91d270
Author: Omer Peleg 
Date:   Wed Apr 20 11:34:11 2016 +0300

iommu/iova: introduce per-cpu caching to iova allocation

The ath9k log you provided has a DMA warning and iommu problems can
cause DMA problems but I cannot make any conclusions yet. To confirm
that this commit really is the problem you could try to revert it with
'git revert -n 9257b4a206fc0229dd5f84b78e4d1ebf3f91d270'. For some
reason I got conflicts but if you are good enough with C you could try
to fix those yourself. Another option is that you disable iommu and see
if that helps.

I'm adding more people and mailing lists related to this commit,
hopefully they have better ideas.

This is Valerio's bisect log:

git bisect start
# good: [2dcd0af568b0cf583645c8a317dd12e344b1c72a] Linux 4.6
git bisect good 2dcd0af568b0cf583645c8a317dd12e344b1c72a
# bad: [523d939ef98fd712632d93a5a2b588e477a7565e] Linux 4.7
git bisect bad 523d939ef98fd712632d93a5a2b588e477a7565e
# good: [0694f0c9e20c47063e4237e5f6649ae5ce5a369a] radix tree test suite: 
remove dependencies on height
git bisect good 0694f0c9e20c47063e4237e5f6649ae5ce5a369a
# good: [e4f7bdc2ec0d0dcc27f7d70db27a620dfdc1f697] Merge branch 'for-4.7-zac' 
of git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata
git bisect good e4f7bdc2ec0d0dcc27f7d70db27a620dfdc1f697
# bad: [049ec1b5a76d34a6980cccdb7c0baeb4eed7a993] Merge tag 'drm-fixes-for-
v4.7-rc2' of git://people.freedesktop.org/~airlied/linux
git bisect bad 049ec1b5a76d34a6980cccdb7c0baeb4eed7a993
# good: [a10c38a4f385f5d7c173a263ff6bb2d36021b3bb] Merge branch 'for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/sage/ceph-client
git bisect

Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms

2016-09-08 Thread Scott Wood 
On Tue, 2016-09-06 at 16:28 +0800, Yangbo Lu wrote:
> The global utilities block controls power management, I/O device
> enabling, power-onreset(POR) configuration monitoring, alternate
> function selection for multiplexed signals,and clock control.
> 
> This patch adds a driver to manage and access global utilities block.
> Initially only reading SVR and registering soc device are supported.
> Other guts accesses, such as reading RCW, should eventually be moved
> into this driver as well.
> 
> Signed-off-by: Yangbo Lu 
> Signed-off-by: Scott Wood 

Don't put my signoff on patches that I didn't put it on myself.  Definitely
don't put mine *after* yours on patches that were last modified by you.

If you want to mention that the soc_id encoding was my suggestion, then do so
explicitly.

> +/* SoC attribute definition for QorIQ platform */
> +static const struct soc_device_attribute qoriq_soc[] = {
> +#ifdef CONFIG_PPC
> + /*
> +  * Power Architecture-based SoCs T Series
> +  */
> +
> + /* SoC: T1024/T1014/T1023/T1013 Rev: 1.0 */
> + { .soc_id   = "svr:0x85400010,name:T1024,die:T1024",
> +   .revision = "1.0",
> + },
> + { .soc_id   = "svr:0x85480010,name:T1024E,die:T1024",
> +   .revision = "1.0",
> + },

Revision could be computed from the low 8 bits of SVR (just as you do for 
unknown SVRs).

We could move the die name into .family:

{
.soc_id = "svr:0x85490010,name:T1023E,",
.family = "QorIQ T1024",
}

I see you dropped svre (and the trailing comma), though I guess the vast
majority of potential users will be looking at .family.  In which case do we
even need name?  If we just make the soc_id be "svr:0x" then we could
shrink the table to an svr+mask that identifies each die.  I'd still want to
keep the "svr:" even if we're giving up on the general tagging system, to make
it clear what the number refers to, and to provide some defense against users
who match only against soc_id rather than soc_id+family.  Or we could go
further and format soc_id as "QorIQ SVR 0x" so that soc_id-only
matches are fully acceptable rather than just less dangerous.

> +static const struct soc_device_attribute *fsl_soc_device_match(
> + unsigned int svr, const struct soc_device_attribute *matches)
> +{
> + char svr_match[50];
> + int n;
> +
> + n = sprintf(svr_match, "*%08x*", svr);

n = sprintf(svr_match, "svr:0x%08x,*", svr);

(according to the current encoding)

> +
> + do {
> + if (!matches->soc_id)
> + return NULL;
> + if (glob_match(svr_match, matches->soc_id))
> + break;
> + } while (matches++);

Are you expecting "matches++" to ever evaluate as false?

> + /* Register soc device */
> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr) {
> + ret = -ENOMEM;
> + goto out_unmap;
> + }

Couldn't this be statically allocated?

> +
> + machine = of_flat_dt_get_machine_name();
> + if (machine)
> + soc_dev_attr->machine = kasprintf(GFP_KERNEL, "%s",
> machine);
> +
> + soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ");
> +
> + svr = fsl_guts_get_svr();
> + fsl_soc = fsl_soc_device_match(svr, qoriq_soc);
> + if (fsl_soc) {
> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s",
> +  fsl_soc->soc_id);

You can use kstrdup() if you're just copying the string as is.

> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%s",
> +    fsl_soc->revision);
> + } else {
> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "0x%08x",
> svr);

kasprintf(GFP_KERNEL, "svr:0x%08x,", svr);


> +
> + soc_dev = soc_device_register(soc_dev_attr);
> + if (IS_ERR(soc_dev)) {
> + ret = -ENODEV;

Why are you changing the error code?

> + goto out;
> + } else {

Unnecessary "else".

> + pr_info("Detected: %s\n", soc_dev_attr->machine);

Machine: %s

> + pr_info("Detected SoC family: %s\n", soc_dev_attr->family);
> + pr_info("Detected SoC ID: %s, revision: %s\n",
> + soc_dev_attr->soc_id, soc_dev_attr->revision);

s/Detected //g


> + }
> + return 0;
> +out:
> + kfree(soc_dev_attr->machine);
> + kfree(soc_dev_attr->family);
> + kfree(soc_dev_attr->soc_id);
> + kfree(soc_dev_attr->revision);
> + kfree(soc_dev_attr);
> +out_unmap:
> + iounmap(guts->regs);
> +out_free:
> + kfree(guts);

devm

> +static int fsl_guts_remove(struct platform_device *dev)
> +{
> + kfree(soc_dev_attr->machine);
> + kfree(soc_dev_attr->family);
> + kfree(soc_dev_attr->soc_id);
> + kfree(soc_dev_attr->revision);
> + kfree(soc_dev_attr);
> +