* Re: [PATCH 1/3] lib/test_kasan: Add bitops tests
From: Mark Rutland @ 2019-05-28 16:50 UTC (permalink / raw)
To: Marco Elver
Cc: peterz, aryabinin, dvyukov, glider, andreyknvl, corbet, tglx,
mingo, bp, hpa, x86, arnd, jpoimboe, linux-doc, linux-kernel,
linux-arch, kasan-dev
In-Reply-To: <20190528163258.260144-1-elver@google.com>
Hi,
On Tue, May 28, 2019 at 06:32:56PM +0200, Marco Elver wrote:
> +static noinline void __init kasan_bitops(void)
> +{
> + long bits = 0;
> + const long bit = sizeof(bits) * 8;
You can use BITS_PER_LONG here.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH 2/3] tools/objtool: add kasan_check_* to uaccess whitelist
From: Peter Zijlstra @ 2019-05-28 17:19 UTC (permalink / raw)
To: Marco Elver
Cc: aryabinin, dvyukov, glider, andreyknvl, corbet, tglx, mingo, bp,
hpa, x86, arnd, jpoimboe, linux-doc, linux-kernel, linux-arch,
kasan-dev
In-Reply-To: <20190528163258.260144-2-elver@google.com>
On Tue, May 28, 2019 at 06:32:57PM +0200, Marco Elver wrote:
> This is a pre-requisite for enabling bitops instrumentation. Some bitops
> may safely be used with instrumentation in uaccess regions.
>
> For example, on x86, `test_bit` is used to test a CPU-feature in a
> uaccess region: arch/x86/ia32/ia32_signal.c:361
That one can easily be moved out of the uaccess region. Any else?
> Signed-off-by: Marco Elver <elver@google.com>
> ---
> tools/objtool/check.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 172f99195726..eff0e5209402 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -443,6 +443,8 @@ static void add_ignores(struct objtool_file *file)
> static const char *uaccess_safe_builtin[] = {
> /* KASAN */
> "kasan_report",
> + "kasan_check_read",
> + "kasan_check_write",
> "check_memory_region",
> /* KASAN out-of-line */
> "__asan_loadN_noabort",
> --
> 2.22.0.rc1.257.g3120a18244-goog
>
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Joel Fernandes @ 2019-05-28 19:00 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Steven Rostedt, linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul Mackerras, rcu
In-Reply-To: <20190528122447.GS28207@linux.ibm.com>
On Tue, May 28, 2019 at 05:24:47AM -0700, Paul E. McKenney wrote:
> On Sat, May 25, 2019 at 02:14:07PM -0400, Joel Fernandes wrote:
> > On Sat, May 25, 2019 at 08:50:35AM -0700, Paul E. McKenney wrote:
> > > On Sat, May 25, 2019 at 10:19:54AM -0400, Joel Fernandes wrote:
> > > > On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote:
> > > > > On Sat, 25 May 2019 04:14:44 -0400
> > > > > Joel Fernandes <joel@joelfernandes.org> wrote:
> > > > >
> > > > > > > I guess the difference between the _raw_notrace and just _raw variants
> > > > > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> > > > > > > that check?
> > > > > >
> > > > > > This is true.
> > > > > >
> > > > > > Since the users of _raw_notrace are very few, is it worth keeping this API
> > > > > > just for sparse checking? The API naming is also confusing. I was expecting
> > > > > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
> > > > > > want to nuke _raw_notrace as done in this series and later we can introduce a
> > > > > > sparse checking version of _raw if need-be. The other option could be to
> > > > > > always do sparse checking for _raw however that used to be the case and got
> > > > > > changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
> > > > >
> > > > > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ?
> > > >
> > > > That would also mean changing 160 usages of _raw to _raw_nocheck in the
> > > > kernel :-/.
> > > >
> > > > The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call
> > > > rcu_check_sparse directly in the calling code for those and eliminate the APIs?
> > > >
> > > > I wonder what Paul thinks about the matter as well.
> > >
> > > My thought is that it is likely that a goodly number of the current uses
> > > of _raw should really be some form of _check, with lockdep expressions
> > > spelled out. Not that working out what exactly those lockdep expressions
> > > should be is necessarily a trivial undertaking. ;-)
> >
> > Yes, currently where I am a bit stuck is the rcu_dereference_raw()
> > cannot possibly know what SRCU domain it is under, so lockdep cannot check if
> > an SRCU lock is held without the user also passing along the SRCU domain. I
> > am trying to change lockdep to see if it can check if *any* srcu domain lock
> > is held (regardless of which one) and complain if none are. This is at least
> > better than no check at all.
> >
> > However, I think it gets tricky for mutexes. If you have something like:
> > mutex_lock(some_mutex);
> > p = rcu_dereference_raw(gp);
> > mutex_unlock(some_mutex);
> >
> > This might be a perfectly valid invocation of _raw, however my checks (patch
> > is still cooking) trigger a lockdep warning becase _raw cannot know that this
> > is Ok. lockdep thinks it is not in a reader section. This then gets into the
> > territory of a new rcu_derference_raw_protected(gp, assert_held(some_mutex))
> > which sucks because its yet another API. To circumvent this issue, can we
> > just have callers of rcu_dereference_raw ensure that they call
> > rcu_read_lock() if they are protecting dereferences by a mutex? That would
> > make things a lot easier and also may be Ok since rcu_read_lock is quite
> > cheap.
>
> Why not just rcu_dereference_protected(lockdep_is_held(some_mutex))?
> The API is already there, and no need for spurious readers.
Hmm, so I gave a bad example, here is a better example:
fib_get_table calls hlist_for_each_entry_rcu()
hlist_for_each_entry_rcu calls rcu_dereference_raw().
This is perfectly Ok to be called under rtnl_mutex. However rcu_dererence_raw
in hlist_for_each_entry_rcu has no way of knowing that the rtnl_mutex held is
sufficient for the protection since it is not directly called by the caller.
I am almost sure I saw other examples of rcu_dereference_raw being called
this way as well.
I was trying to make an "automatic" lockdep check for all this, but it is
quite hard to do so without passing down lockdep experessions down a call
chain thus complicating all such callchains.
Further I don't think code can trivially be converted from
rcu_dereference_raw to rcu_dereference_protected even if the protection being
offered is known, since the former does not do sparse checking and the latter
might trigger false sparse checks in case the pointer in concern is protected
both by RCU and non-RCU methods. I believe this is why you removed sparse
checking from rcu_dereference_raw as well:
http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
> > > That aside, if we are going to change the name of an API that is
> > > used 160 places throughout the tree, we would need to have a pretty
> > > good justification. Without such a justification, it will just look
> > > like pointless churn to the various developers and maintainers on the
> > > receiving end of the patches.
> >
> > Actually, the API name change is not something I want to do, it is Steven
> > suggestion. My suggestion is let us just delete _raw_notrace and just use the
> > _raw API for tracing, since _raw doesn't do any tracing anyway. Steve pointed
> > that _raw_notrace does sparse checking unlike _raw, but I think that isn't an
> > issue since _raw doesn't do such checking at the moment anyway.. (if possible
> > check my cover letter again for details/motivation of this series).
>
> Understood, but regardless of who suggested it, if we are to go through
> with it, good justification will be required. ;-)
Ok ;-). About the names of the APIs, I thought of leaving rcu_dereference_raw
and its callers intact, and just rename:
* hlist_for_each_entry_rcu_notrace
* rcu_dereference_raw_notrace
to:
* hlist_for_each_entry_rcu_sparse
* rcu_dereference_raw_sparse
The _sparse would stand for "sparse checking". However I am open to better
names..
Such renaming would avoid confusion and keep the fact about sparse checking
less ambiguous.
thanks!
- Joel
^ permalink raw reply
* Re: [PATCH v10 01/12] MODSIGN: Export module signature definitions
From: Thiago Jung Bauermann @ 2019-05-28 19:03 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557416528.10635.62.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>> IMA will use the module_signature format for append signatures, so export
>> the relevant definitions and factor out the code which verifies that the
>> appended signature trailer is valid.
>>
>> Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
>> and be able to use mod_check_sig() without having to depend on either
>> CONFIG_MODULE_SIG or CONFIG_MODULES.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> Cc: Jessica Yu <jeyu@kernel.org>
>
> Just a couple minor questions/comments below.
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Thanks for your review and your comments!
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 4592bf7997c0..a71019553ee1 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1906,7 +1906,7 @@ config MODULE_SRCVERSION_ALL
>> config MODULE_SIG
>> bool "Module signature verification"
>> depends on MODULES
>> - select SYSTEM_DATA_VERIFICATION
>> + select MODULE_SIG_FORMAT
>> help
>> Check modules for valid signatures upon load: the signature
>> is simply appended to the module. For more information see
>> @@ -2036,6 +2036,10 @@ config TRIM_UNUSED_KSYMS
>>
>> endif # MODULES
>>
>> +config MODULE_SIG_FORMAT
>> + def_bool n
>> + select SYSTEM_DATA_VERIFICATION
>
> Normally Kconfigs, in the same file, are defined before they are used.
> I'm not sure if that is required or just a convention.
I think it's a convention, because it seemed to work in the current way.
For the next version I moved the config MODULE_SIG_FORMAT definition to
just before "menuconfig MODULES"
>> diff --git a/kernel/module_signature.c b/kernel/module_signature.c
>> new file mode 100644
>> index 000000000000..6d5e59f27f55
>> --- /dev/null
>> +++ b/kernel/module_signature.c
>> @@ -0,0 +1,45 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Module signature checker
>> + *
>> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
>> + * Written by David Howells (dhowells@redhat.com)
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/printk.h>
>> +#include <linux/module_signature.h>
>> +#include <asm/byteorder.h>
>> +
>> +/**
>> + * mod_check_sig - check that the given signature is sane
>> + *
>> + * @ms: Signature to check.
>> + * @file_len: Size of the file to which @ms is appended.
>
> "name" is missing.
Fixed.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 11/12] ima: Define ima-modsig template
From: Thiago Jung Bauermann @ 2019-05-28 19:09 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557442889.10635.88.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>> Define new "d-modsig" template field which holds the digest that is
>> expected to match the one contained in the modsig, and also new "modsig"
>> template field which holds the appended file signature.
>>
>> Add a new "ima-modsig" defined template descriptor with the new fields as
>> well as the ones from the "ima-sig" descriptor.
>>
>> Change ima_store_measurement() to accept a struct modsig * argument so that
>> it can be passed along to the templates via struct ima_event_data.
>>
>> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
> Thanks, Roberto. Just some thoughts inline below.
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Thanks!
>> +/*
>> + * Validating the appended signature included in the measurement list requires
>> + * the file hash calculated without the appended signature (i.e., the 'd-modsig'
>> + * field). Therefore, notify the user if they have the 'modsig' field but not
>> + * the 'd-modsig' field in the template.
>> + */
>> +static void check_current_template_modsig(void)
>> +{
>> +#define MSG "template with 'modsig' field also needs 'd-modsig' field\n"
>> + struct ima_template_desc *template;
>> + bool has_modsig, has_dmodsig;
>> + static bool checked;
>> + int i;
>> +
>> + /* We only need to notify the user once. */
>> + if (checked)
>> + return;
>> +
>> + has_modsig = has_dmodsig = false;
>> + template = ima_template_desc_current();
>> + for (i = 0; i < template->num_fields; i++) {
>> + if (!strcmp(template->fields[i]->field_id, "modsig"))
>> + has_modsig = true;
>> + else if (!strcmp(template->fields[i]->field_id, "d-modsig"))
>> + has_dmodsig = true;
>> + }
>> +
>> + if (has_modsig && !has_dmodsig)
>> + pr_notice(MSG);
>> +
>> + checked = true;
>> +#undef MSG
>> +}
>> +
>
> There was some recent discussion about supporting per IMA policy rule
> template formats. This feature will allow just the kexec kernel image
> to require ima-modsig. When per policy rule template formats support
> is upstreamed, this function will need to be updated.
Indeed. Thanks for the clarification. For the next iteration I rebased
on top of Matthew Garret's "IMA: Allow profiles to define the desired
IMA template" patch. I'm currently adapting this check accordingly.
>> @@ -389,3 +425,25 @@ int ima_eventsig_init(struct ima_event_data *event_data,
>> return ima_write_template_field_data(xattr_value, event_data->xattr_len,
>> DATA_FMT_HEX, field_data);
>> }
>> +
>> +int ima_eventmodsig_init(struct ima_event_data *event_data,
>> + struct ima_field_data *field_data)
>> +{
>> + const void *data;
>> + u32 data_len;
>> + int rc;
>> +
>> + if (!event_data->modsig)
>> + return 0;
>> +
>> + /*
>> + * The xattr_value for IMA_MODSIG is a runtime structure containing
>> + * pointers. Get its raw data instead.
>> + */
>
> "xattr_value"? The comment needs some clarification.
Oops, forgot to update this comment. This is the new version:
/*
* modsig is a runtime structure containing pointers. Get its raw data
* instead.
*/
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 12/12] ima: Store the measurement again when appraising a modsig
From: Thiago Jung Bauermann @ 2019-05-28 19:14 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1559052560.4090.14.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> Hi Thiago,
>
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>> If the IMA template contains the "modsig" or "d-modsig" field, then the
>> modsig should be added to the measurement list when the file is appraised.
>>
>> And that is what normally happens, but if a measurement rule caused a file
>> containing a modsig to be measured before a different rule causes it to be
>> appraised, the resulting measurement entry will not contain the modsig
>> because it is only fetched during appraisal. When the appraisal rule
>> triggers, it won't store a new measurement containing the modsig because
>> the file was already measured.
>>
>> We need to detect that situation and store an additional measurement with
>> the modsig. This is done by adding an IMA_MEASURE action flag if we read a
>> modsig and the IMA template contains a modsig field.
>
> With the new per policy rule "template" support being added, this
> patch needs to be modified so that the per policy "template" format is
> checked. ima_template_has_modsig() should be called with the
> template_desc being used.
Right. Thanks for point out what needs to be done. After rebasing on top
of Matthew Garret's "IMA: Allow profiles to define the desired IMA
template" patch I changed ima_template_has_modsig() to check the
template_desc obtained from process_measurement().
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Thiago Jung Bauermann @ 2019-05-28 19:23 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557442868.10635.87.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> Hi Thiago,
>
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index fca7a3f23321..a7a20a8c15c1 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -1144,6 +1144,12 @@ void ima_delete_rules(void)
>> }
>> }
>>
>> +#define __ima_hook_stringify(str) (#str),
>> +
>> +const char *const func_tokens[] = {
>> + __ima_hooks(__ima_hook_stringify)
>> +};
>> +
>> #ifdef CONFIG_IMA_READ_POLICY
>> enum {
>> mask_exec = 0, mask_write, mask_read, mask_append
>> @@ -1156,12 +1162,6 @@ static const char *const mask_tokens[] = {
>> "MAY_APPEND"
>> };
>>
>> -#define __ima_hook_stringify(str) (#str),
>> -
>> -static const char *const func_tokens[] = {
>> - __ima_hooks(__ima_hook_stringify)
>> -};
>> -
>> void *ima_policy_start(struct seq_file *m, loff_t *pos)
>> {
>> loff_t l = *pos;
>
> Is moving this something left over from previous versions or there is
> a need for this change?
Well, it's not a strong need, but it's still relevant in the current
version. I use func_tokens in ima_read_modsig() in order to be able to
mention the hook name in mod_check_sig()'s error message:
In ima_read_modsig():
rc = mod_check_sig(sig, buf_len, func_tokens[func]);
And in mod_check_sig():
pr_err("%s: Module is not signed with expected PKCS#7 message\n",
name);
If you think it's not worth it to expose func_tokens, I can make
ima_read_modsig() pass a more generic const string such as "IMA modsig"
for example.
> Other than this, the patch looks good.
Nice!
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Thiago Jung Bauermann @ 2019-05-28 19:27 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <1557835765.4139.9.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> Hi Thiago,
>
> On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
>>
>> @@ -326,6 +356,10 @@ int ima_appraise_measurement(enum ima_hooks func,
>> case INTEGRITY_UNKNOWN:
>> break;
>> case INTEGRITY_NOXATTRS:/* No EVM protected xattrs. */
>> +/* It's fine not to have xattrs when using a modsig. */
>> +if (try_modsig)
>> +break;
>> +/* fall through */
>> case INTEGRITY_NOLABEL:/* No security.evm xattr. */
>> cause = "missing-HMAC";
>> goto out;
>> @@ -340,6 +374,14 @@ int ima_appraise_measurement(enum ima_hooks func,
>> rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
>> &cause);
>>
>> +/*
>> + * If we have a modsig and either no imasig or the imasig's key isn't
>> + * known, then try verifying the modsig.
>> + */
>> +if (status != INTEGRITY_PASS && try_modsig &&
>> + (!xattr_value || rc == -ENOKEY))
>> +rc = modsig_verify(func, modsig, &status, &cause);
>
> EVM protects other security xattrs, not just security.ima, if they
> exist. As a result, evm_verifyxattr() could pass based on the other
> security xattrs.
Indeed! It doesn't make sense to test for status != INTEGRITY_PASS here.
Not sure what I was thinking. Thanks for spotting it. With your other
comments about this if clause, this code now reads:
/*
* If we have a modsig and either no imasig or the imasig's key isn't
* known, then try verifying the modsig.
*/
if (try_modsig &&
(!xattr_value || xattr_value->type == IMA_XATTR_DIGEST_NG ||
rc == -ENOKEY))
rc = modsig_verify(func, modsig, &status, &cause);
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH RFC 0/5] Remove some notrace RCU APIs
From: Paul E. McKenney @ 2019-05-28 20:00 UTC (permalink / raw)
To: Joel Fernandes
Cc: Steven Rostedt, linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Miguel Ojeda,
Paul Mackerras, rcu
In-Reply-To: <20190528190007.GC252809@google.com>
On Tue, May 28, 2019 at 03:00:07PM -0400, Joel Fernandes wrote:
> On Tue, May 28, 2019 at 05:24:47AM -0700, Paul E. McKenney wrote:
> > On Sat, May 25, 2019 at 02:14:07PM -0400, Joel Fernandes wrote:
> > > On Sat, May 25, 2019 at 08:50:35AM -0700, Paul E. McKenney wrote:
> > > > On Sat, May 25, 2019 at 10:19:54AM -0400, Joel Fernandes wrote:
> > > > > On Sat, May 25, 2019 at 07:08:26AM -0400, Steven Rostedt wrote:
> > > > > > On Sat, 25 May 2019 04:14:44 -0400
> > > > > > Joel Fernandes <joel@joelfernandes.org> wrote:
> > > > > >
> > > > > > > > I guess the difference between the _raw_notrace and just _raw variants
> > > > > > > > is that _notrace ones do a rcu_check_sparse(). Don't we want to keep
> > > > > > > > that check?
> > > > > > >
> > > > > > > This is true.
> > > > > > >
> > > > > > > Since the users of _raw_notrace are very few, is it worth keeping this API
> > > > > > > just for sparse checking? The API naming is also confusing. I was expecting
> > > > > > > _raw_notrace to do fewer checks than _raw, instead of more. Honestly, I just
> > > > > > > want to nuke _raw_notrace as done in this series and later we can introduce a
> > > > > > > sparse checking version of _raw if need-be. The other option could be to
> > > > > > > always do sparse checking for _raw however that used to be the case and got
> > > > > > > changed in http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
> > > > > >
> > > > > > What if we just rename _raw to _raw_nocheck, and _raw_notrace to _raw ?
> > > > >
> > > > > That would also mean changing 160 usages of _raw to _raw_nocheck in the
> > > > > kernel :-/.
> > > > >
> > > > > The tracing usage of _raw_notrace is only like 2 or 3 users. Can we just call
> > > > > rcu_check_sparse directly in the calling code for those and eliminate the APIs?
> > > > >
> > > > > I wonder what Paul thinks about the matter as well.
> > > >
> > > > My thought is that it is likely that a goodly number of the current uses
> > > > of _raw should really be some form of _check, with lockdep expressions
> > > > spelled out. Not that working out what exactly those lockdep expressions
> > > > should be is necessarily a trivial undertaking. ;-)
> > >
> > > Yes, currently where I am a bit stuck is the rcu_dereference_raw()
> > > cannot possibly know what SRCU domain it is under, so lockdep cannot check if
> > > an SRCU lock is held without the user also passing along the SRCU domain. I
> > > am trying to change lockdep to see if it can check if *any* srcu domain lock
> > > is held (regardless of which one) and complain if none are. This is at least
> > > better than no check at all.
> > >
> > > However, I think it gets tricky for mutexes. If you have something like:
> > > mutex_lock(some_mutex);
> > > p = rcu_dereference_raw(gp);
> > > mutex_unlock(some_mutex);
> > >
> > > This might be a perfectly valid invocation of _raw, however my checks (patch
> > > is still cooking) trigger a lockdep warning becase _raw cannot know that this
> > > is Ok. lockdep thinks it is not in a reader section. This then gets into the
> > > territory of a new rcu_derference_raw_protected(gp, assert_held(some_mutex))
> > > which sucks because its yet another API. To circumvent this issue, can we
> > > just have callers of rcu_dereference_raw ensure that they call
> > > rcu_read_lock() if they are protecting dereferences by a mutex? That would
> > > make things a lot easier and also may be Ok since rcu_read_lock is quite
> > > cheap.
> >
> > Why not just rcu_dereference_protected(lockdep_is_held(some_mutex))?
> > The API is already there, and no need for spurious readers.
>
> Hmm, so I gave a bad example, here is a better example:
>
> fib_get_table calls hlist_for_each_entry_rcu()
> hlist_for_each_entry_rcu calls rcu_dereference_raw().
>
> This is perfectly Ok to be called under rtnl_mutex. However rcu_dererence_raw
> in hlist_for_each_entry_rcu has no way of knowing that the rtnl_mutex held is
> sufficient for the protection since it is not directly called by the caller.
Agreed, and this just happens to be one of the use cases that led to
rcu_dereference_raw(). The calling code (in this case, FIB) simply has
no idea what the synchronization strategy might be.
> I am almost sure I saw other examples of rcu_dereference_raw being called
> this way as well.
And I am OK with this sort of use case. The ones I am less happy with
are the ones where there really is a lockdep expression that could be
constructed.
> I was trying to make an "automatic" lockdep check for all this, but it is
> quite hard to do so without passing down lockdep experessions down a call
> chain thus complicating all such callchains.
Understood! Not an easy task.
> Further I don't think code can trivially be converted from
> rcu_dereference_raw to rcu_dereference_protected even if the protection being
> offered is known, since the former does not do sparse checking and the latter
> might trigger false sparse checks in case the pointer in concern is protected
> both by RCU and non-RCU methods. I believe this is why you removed sparse
> checking from rcu_dereference_raw as well:
>
> http://lists.infradead.org/pipermail/linux-afs/2016-July/001016.html
Good point!
> > > > That aside, if we are going to change the name of an API that is
> > > > used 160 places throughout the tree, we would need to have a pretty
> > > > good justification. Without such a justification, it will just look
> > > > like pointless churn to the various developers and maintainers on the
> > > > receiving end of the patches.
> > >
> > > Actually, the API name change is not something I want to do, it is Steven
> > > suggestion. My suggestion is let us just delete _raw_notrace and just use the
> > > _raw API for tracing, since _raw doesn't do any tracing anyway. Steve pointed
> > > that _raw_notrace does sparse checking unlike _raw, but I think that isn't an
> > > issue since _raw doesn't do such checking at the moment anyway.. (if possible
> > > check my cover letter again for details/motivation of this series).
> >
> > Understood, but regardless of who suggested it, if we are to go through
> > with it, good justification will be required. ;-)
>
> Ok ;-). About the names of the APIs, I thought of leaving rcu_dereference_raw
> and its callers intact, and just rename:
>
> * hlist_for_each_entry_rcu_notrace
> * rcu_dereference_raw_notrace
>
> to:
> * hlist_for_each_entry_rcu_sparse
> * rcu_dereference_raw_sparse
>
> The _sparse would stand for "sparse checking". However I am open to better
> names..
>
> Such renaming would avoid confusion and keep the fact about sparse checking
> less ambiguous.
Let's give people a few days to propose different names, and if nothing
compelling, those names look good. There are not very many of them, so
the penalty for having to rename is quite low.
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Mimi Zohar @ 2019-05-28 20:06 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-integrity, linux-security-module, keyrings, linux-crypto,
linuxppc-dev, linux-doc, linux-kernel, Dmitry Kasatkin,
James Morris, Serge E. Hallyn, David Howells, David Woodhouse,
Jessica Yu, Herbert Xu, David S. Miller, Jonathan Corbet,
AKASHI, Takahiro
In-Reply-To: <87zhn65qor.fsf@morokweng.localdomain>
On Tue, 2019-05-28 at 16:23 -0300, Thiago Jung Bauermann wrote:
> Mimi Zohar <zohar@linux.ibm.com> writes:
>
> > Hi Thiago,
> >
> >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> >> index fca7a3f23321..a7a20a8c15c1 100644
> >> --- a/security/integrity/ima/ima_policy.c
> >> +++ b/security/integrity/ima/ima_policy.c
> >> @@ -1144,6 +1144,12 @@ void ima_delete_rules(void)
> >> }
> >> }
> >>
> >> +#define __ima_hook_stringify(str) (#str),
> >> +
> >> +const char *const func_tokens[] = {
> >> + __ima_hooks(__ima_hook_stringify)
> >> +};
> >> +
> >> #ifdef CONFIG_IMA_READ_POLICY
> >> enum {
> >> mask_exec = 0, mask_write, mask_read, mask_append
> >> @@ -1156,12 +1162,6 @@ static const char *const mask_tokens[] = {
> >> "MAY_APPEND"
> >> };
> >>
> >> -#define __ima_hook_stringify(str) (#str),
> >> -
> >> -static const char *const func_tokens[] = {
> >> - __ima_hooks(__ima_hook_stringify)
> >> -};
> >> -
> >> void *ima_policy_start(struct seq_file *m, loff_t *pos)
> >> {
> >> loff_t l = *pos;
> >
> > Is moving this something left over from previous versions or there is
> > a need for this change?
>
> Well, it's not a strong need, but it's still relevant in the current
> version. I use func_tokens in ima_read_modsig() in order to be able to
> mention the hook name in mod_check_sig()'s error message:
>
> In ima_read_modsig():
>
> rc = mod_check_sig(sig, buf_len, func_tokens[func]);
>
> And in mod_check_sig():
>
> pr_err("%s: Module is not signed with expected PKCS#7 message\n",
> name);
>
> If you think it's not worth it to expose func_tokens, I can make
> ima_read_modsig() pass a more generic const string such as "IMA modsig"
> for example.
This is fine. I somehow missed moving func_tokens[] outside of the
ifdef was in order to make it independent of "CONFIG_IMA_READ_POLICY".
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v2 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: Dave Chiluk @ 2019-05-28 22:25 UTC (permalink / raw)
To: Peter Oskolkov
Cc: Phil Auld, Peter Zijlstra, Ingo Molnar, cgroups,
Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
linux-doc, Ben Segall
In-Reply-To: <CAFTs51VhpDk9iW5UT62CkPCN3SjgUHHO1nVqhe+ssHMYqou6Bg@mail.gmail.com>
On Fri, May 24, 2019 at 5:07 PM Peter Oskolkov <posk@posk.io> wrote:
> Linux CPU scheduling tail latency is a well-known issue and a major
> pain point in some workloads:
> https://www.google.com/search?q=linux+cpu+scheduling+tail+latency
>
> Even assuming that nobody noticed this particular cause
> of CPU scheduling latencies, it does not mean the problem should be waved
> away. At least it should be documented, if at this point it decided that
> it is difficult to address it in a meaningful way. And, preferably, a way
> to address the issue later on should be discussed and hopefully agreed to.
Pursuing reducing tail latencies for our web application is the
precise reason I created this patch set. Those applications that
previously were responding in 20ms 95% where now taking 220ms. Those
were correctly sized applications prior to 512ac999. After which, they
started seeing massive increases in their latencies due to hitting
throttling with lower than quota amounts of cpu usage.
I'll see if I can rework the documentation. Any specific
suggestions for how that can be worded would be appreciated.
^ permalink raw reply
* Re: [PATCH 1/3] xen: remove tmem driver
From: Boris Ostrovsky @ 2019-05-28 22:44 UTC (permalink / raw)
To: Juergen Gross, linux-kernel, linux-doc
Cc: Jonathan Corbet, Stefano Stabellini, xen-devel
In-Reply-To: <20190527103207.13287-2-jgross@suse.com>
> diff --git a/include/xen/balloon.h b/include/xen/balloon.h
> index 4914b93a23f2..a72ef3f88b39 100644
> --- a/include/xen/balloon.h
> +++ b/include/xen/balloon.h
> @@ -28,14 +28,6 @@ int alloc_xenballooned_pages(int nr_pages, struct page **pages);
> void free_xenballooned_pages(int nr_pages, struct page **pages);
>
> struct device;
This can be removed as well.
Other than that,
Acked-by: Boris Ostriovsky <boris.ostrovsky@oracle.com>
> -#ifdef CONFIG_XEN_SELFBALLOONING
> -extern int register_xen_selfballooning(struct device *dev);
> -#else
> -static inline int register_xen_selfballooning(struct device *dev)
> -{
> - return -ENOSYS;
> -}
> -#endif
>
> #ifdef CONFIG_XEN_BALLOON
> void xen_balloon_init(void);
>
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Paolo Bonzini @ 2019-05-29 1:24 UTC (permalink / raw)
To: Tao Xu, rkrcmar, corbet, tglx, mingo, bp, hpa,
sean.j.christopherson
Cc: x86, kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <20190524075637.29496-2-tao3.xu@intel.com>
On 24/05/19 09:56, Tao Xu wrote:
> +7.19 KVM_CAP_ENABLE_USR_WAIT_PAUSE
> +
> +Architectures: x86
> +Parameters: args[0] whether feature should be enabled or not
> +
> +With this capability enabled, a VM can use UMONITOR, UMWAIT and TPAUSE
> +instructions. If the instruction causes a delay, the amount of
> +time delayed is called here the physical delay. The physical delay is
> +first computed by determining the virtual delay (the time to delay
> +relative to the VM’s timestamp counter). Otherwise, UMONITOR, UMWAIT
> +and TPAUSE cause an invalid-opcode exception(#UD).
> +
There is no need to make it a capability. You can just check the guest
CPUID and see if it includes X86_FEATURE_WAITPKG.
Paolo
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Paolo Bonzini @ 2019-05-29 1:25 UTC (permalink / raw)
To: Peter Zijlstra, Tao Xu
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <20190527103003.GX2623@hirez.programming.kicks-ass.net>
On 27/05/19 12:30, Peter Zijlstra wrote:
>> This patch adds support for UMONITOR, UMWAIT and TPAUSE instructions
>> in kvm, and by default dont't expose it to kvm and provide a capability
>> to enable it.
>
> I'm thinking this should be conditional on the guest being a 1:1 guest,
> and I also seem to remember we have bits for that already -- they were
> used to disable paravirt spinlocks for example.
This should be userspace's choice. It would indeed be silly to enable
this while overcommitted, but KVM doesn't really care.
Paolo
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Paolo Bonzini @ 2019-05-29 1:26 UTC (permalink / raw)
To: Tao Xu, Wanpeng Li
Cc: Peter Zijlstra, Radim Krcmar, Jonathan Corbet, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, Sean Christopherson,
the arch/x86 maintainers, kvm, linux-doc, LKML, jingqi.liu
In-Reply-To: <072dd34e-0361-5a06-4d0b-d04e8150a3bb@intel.com>
On 28/05/19 09:19, Tao Xu wrote:
>
> Thank you! This information really helped me. After I read the code in
> KVM/QEMU, I was wondering that with qemu command-line "-cpu
> host,+kvm-hint-dedicated", then in KVM,
> "kvm_hint_has_feature(KVM_HINTS_DEDICATED)" will be true, am I right?
Yes, but it doesn't matter for this patch series.
Paolo
^ permalink raw reply
* Re: [PATCH v2 3/3] KVM: vmx: handle vm-exit for UMWAIT and TPAUSE
From: Paolo Bonzini @ 2019-05-29 1:28 UTC (permalink / raw)
To: Tao Xu, rkrcmar, corbet, tglx, mingo, bp, hpa,
sean.j.christopherson
Cc: x86, kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <20190524075637.29496-4-tao3.xu@intel.com>
On 24/05/19 09:56, Tao Xu wrote:
> As the latest Intel 64 and IA-32 Architectures Software Developer's
> Manual, UMWAIT and TPAUSE instructions cause a VM exit if the
> “RDTSC exiting” and “enable user wait and pause” VM-execution controls
> are both 1.
>
> This patch is to handle the vm-exit for UMWAIT and TPAUSE as invalid_op.
KVM never enables RDTSC exiting, so this is not necessary.
Paolo
^ permalink raw reply
* Re: [PATCH v2 2/3] KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL
From: Paolo Bonzini @ 2019-05-29 1:29 UTC (permalink / raw)
To: Tao Xu, rkrcmar, corbet, tglx, mingo, bp, hpa,
sean.j.christopherson
Cc: x86, kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <20190524075637.29496-3-tao3.xu@intel.com>
On 24/05/19 09:56, Tao Xu wrote:
> +
> + if (rdmsrl_safe(MSR_IA32_UMWAIT_CONTROL, &host_umwait_control))
> + return;
> +
Does the host value ever change? If not, this can perhaps be read once
when kvm_intel is loaded. And if it changes often, it should be
shadowed into a percpu variable.
Paolo
^ permalink raw reply
* Re: [PATCH v2 2/3] KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL
From: Tao Xu @ 2019-05-29 1:38 UTC (permalink / raw)
To: Paolo Bonzini, rkrcmar, corbet, tglx, mingo, bp, hpa,
sean.j.christopherson
Cc: x86, kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <c9f5050a-6144-adbc-25ef-8a7543176ac6@redhat.com>
On 29/05/2019 09:29, Paolo Bonzini wrote:
> On 24/05/19 09:56, Tao Xu wrote:
>> +
>> + if (rdmsrl_safe(MSR_IA32_UMWAIT_CONTROL, &host_umwait_control))
>> + return;
>> +
>
> Does the host value ever change? If not, this can perhaps be read once
> when kvm_intel is loaded. And if it changes often, it should be
> shadowed into a percpu variable.
>
> Paolo
>
Yes, the host value may change, we contact the host patch author Fenghua
to add the shadow in host when the host msr value change. And we will
improve this in the next version of patch.
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Tao Xu @ 2019-05-29 2:05 UTC (permalink / raw)
To: Paolo Bonzini
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <419f62f3-69a8-7ec0-5eeb-20bed69925f2@redhat.com>
On 29/05/2019 09:24, Paolo Bonzini wrote:
> On 24/05/19 09:56, Tao Xu wrote:
>> +7.19 KVM_CAP_ENABLE_USR_WAIT_PAUSE
>> +
>> +Architectures: x86
>> +Parameters: args[0] whether feature should be enabled or not
>> +
>> +With this capability enabled, a VM can use UMONITOR, UMWAIT and TPAUSE
>> +instructions. If the instruction causes a delay, the amount of
>> +time delayed is called here the physical delay. The physical delay is
>> +first computed by determining the virtual delay (the time to delay
>> +relative to the VM’s timestamp counter). Otherwise, UMONITOR, UMWAIT
>> +and TPAUSE cause an invalid-opcode exception(#UD).
>> +
>
> There is no need to make it a capability. You can just check the guest
> CPUID and see if it includes X86_FEATURE_WAITPKG.
>
> Paolo
>
Thank you Paolo, but I have another question. I was wondering if it is
appropriate to enable X86_FEATURE_WAITPKG when QEMU uses "-overcommit
cpu-pm=on"? Or just enable X86_FEATURE_WAITPKG when QEMU add the feature
"-cpu host,+waitpkg"? User wait instructions is the wait or pause
instructions may be executed at any privilege level, but can use
IA32_UMWAIT_CONTROL to set the maximum time.
^ permalink raw reply
* Re: [PATCH v2 3/3] KVM: vmx: handle vm-exit for UMWAIT and TPAUSE
From: Tao Xu @ 2019-05-29 2:25 UTC (permalink / raw)
To: Paolo Bonzini
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <b0958339-b23c-dd9d-8673-aae098769738@redhat.com>
On 29/05/2019 09:28, Paolo Bonzini wrote:
> On 24/05/19 09:56, Tao Xu wrote:
>> As the latest Intel 64 and IA-32 Architectures Software Developer's
>> Manual, UMWAIT and TPAUSE instructions cause a VM exit if the
>> “RDTSC exiting” and “enable user wait and pause” VM-execution controls
>> are both 1.
>>
>> This patch is to handle the vm-exit for UMWAIT and TPAUSE as invalid_op.
>
> KVM never enables RDTSC exiting, so this is not necessary.
>
> Paolo
>
OK, but should we just drop this patch?
Or add the VMX_EXIT_REASONS bits of UMWAIT and TPAUSE and handle like
XSAVES/XRSTORS:
"kvm_skip_emulated_instruction(vcpu);"
"WARN(1, "this should never happen\n");"
Looking forward to your reply.
Tao
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Paolo Bonzini @ 2019-05-29 2:38 UTC (permalink / raw)
To: Tao Xu
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <c1b27714-2eb8-055e-f26c-e17787d83bb6@intel.com>
On 29/05/19 04:05, Tao Xu wrote:
>>
>
> Thank you Paolo, but I have another question. I was wondering if it is
> appropriate to enable X86_FEATURE_WAITPKG when QEMU uses "-overcommit
> cpu-pm=on"?
"-overcommit" only establishes the behavior of KVM, it doesn't change
the cpuid bits. So you'd need "-cpu" as well.
Paolo
> Or just enable X86_FEATURE_WAITPKG when QEMU add the feature
> "-cpu host,+waitpkg"? User wait instructions is the wait or pause
> instructions may be executed at any privilege level, but can use
> IA32_UMWAIT_CONTROL to set the maximum time.
^ permalink raw reply
* Re: [PATCH v2 3/3] KVM: vmx: handle vm-exit for UMWAIT and TPAUSE
From: Paolo Bonzini @ 2019-05-29 2:39 UTC (permalink / raw)
To: Tao Xu
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <a2b463ee-c032-555e-b012-184e4f4753f1@intel.com>
On 29/05/19 04:25, Tao Xu wrote:
>>
> OK, but should we just drop this patch?
> Or add the VMX_EXIT_REASONS bits of UMWAIT and TPAUSE and handle like
> XSAVES/XRSTORS:
> "kvm_skip_emulated_instruction(vcpu);"
> "WARN(1, "this should never happen\n");"
Yes, this sounds good to me.
Paolo
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Tao Xu @ 2019-05-29 3:12 UTC (permalink / raw)
To: Paolo Bonzini
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <b5daf72d-d764-baa4-8e7f-b09dff417786@redhat.com>
On 5/29/2019 10:38 AM, Paolo Bonzini wrote:
> On 29/05/19 04:05, Tao Xu wrote:
>>>
>>
>> Thank you Paolo, but I have another question. I was wondering if it is
>> appropriate to enable X86_FEATURE_WAITPKG when QEMU uses "-overcommit
>> cpu-pm=on"?
>
> "-overcommit" only establishes the behavior of KVM, it doesn't change
> the cpuid bits. So you'd need "-cpu" as well.
>
> Paolo
>
OK I got it. Thank you for your review.
^ permalink raw reply
* Re: [PATCH 6/8] docs/gpu: fix a documentation build break in i915.rst
From: Daniel Vetter @ 2019-05-29 6:54 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Linux Doc Mailing List, Linux Kernel Mailing List, Jani Nikula,
Markus Heiser, Mauro Carvalho Chehab, Oleksandr Natalenko
In-Reply-To: <20190522205034.25724-7-corbet@lwn.net>
On Wed, May 22, 2019 at 10:51 PM Jonathan Corbet <corbet@lwn.net> wrote:
>
> Documentation/gpu/i915.rst is not included in the TOC tree, but newer
> versions of sphinx parse it anyway. That leads to this hard build failure:
It is included I think: Documentation/gpu/index.rst -> drivers.rst ->
i915.rst. With that corrected A-b: me.
btw this patch didn't go to intel-gfx and all i915 maintainers, I
think per get_maintainers.pl it should have. Just asking since I had a
few patches of my own where get_maintainers.pl didn't seem to do the
right thing somehow.
-Daniel
>
> > Global GTT Fence Handling
> > ~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > reST markup error:
> > /stuff/k/git/kernel/Documentation/gpu/i915.rst:403: (SEVERE/4) Title level inconsistent:
>
> Make the underlining consistent and restore a working docs build.
>
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
> Documentation/gpu/i915.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> index 055df45596c1..cf9ff64753cc 100644
> --- a/Documentation/gpu/i915.rst
> +++ b/Documentation/gpu/i915.rst
> @@ -401,13 +401,13 @@ GTT Fences and Swizzling
> :internal:
>
> Global GTT Fence Handling
> -~~~~~~~~~~~~~~~~~~~~~~~~~
> +-------------------------
>
> .. kernel-doc:: drivers/gpu/drm/i915/i915_gem_fence_reg.c
> :doc: fence register handling
>
> Hardware Tiling and Swizzling Details
> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +-------------------------------------
>
> .. kernel-doc:: drivers/gpu/drm/i915/i915_gem_fence_reg.c
> :doc: tiling swizzling details
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* [PATCH 21/21] EDAC, Documentation: Describe CPER module definition and DIMM ranks
From: Robert Richter @ 2019-05-29 8:44 UTC (permalink / raw)
To: Borislav Petkov, Tony Luck, James Morse, Mauro Carvalho Chehab,
Jonathan Corbet
Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
Robert Richter, linux-doc@vger.kernel.org
In-Reply-To: <20190529084344.28562-1-rrichter@marvell.com>
Update on CPER DIMM naming convention and DIMM ranks.
Signed-off-by: Robert Richter <rrichter@marvell.com>
---
Documentation/admin-guide/ras.rst | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/Documentation/admin-guide/ras.rst b/Documentation/admin-guide/ras.rst
index c7495e42e6f4..4e2a01c77a9c 100644
--- a/Documentation/admin-guide/ras.rst
+++ b/Documentation/admin-guide/ras.rst
@@ -330,9 +330,12 @@ There can be multiple csrows and multiple channels.
.. [#f4] Nowadays, the term DIMM (Dual In-line Memory Module) is widely
used to refer to a memory module, although there are other memory
- packaging alternatives, like SO-DIMM, SIMM, etc. Along this document,
- and inside the EDAC system, the term "dimm" is used for all memory
- modules, even when they use a different kind of packaging.
+ packaging alternatives, like SO-DIMM, SIMM, etc. The UEFI
+ specification (Version 2.7) defines a memory module in the Common
+ Platform Error Record (CPER) section to be an SMBIOS Memory Device
+ (Type 17). Along this document, and inside the EDAC system, the term
+ "dimm" is used for all memory modules, even when they use a
+ different kind of packaging.
Memory controllers allow for several csrows, with 8 csrows being a
typical value. Yet, the actual number of csrows depends on the layout of
@@ -349,12 +352,14 @@ controllers. The following example will assume 2 channels:
| | ``ch0`` | ``ch1`` |
+============+===========+===========+
| ``csrow0`` | DIMM_A0 | DIMM_B0 |
- +------------+ | |
- | ``csrow1`` | | |
+ | | rank0 | rank0 |
+ +------------+ - | - |
+ | ``csrow1`` | rank1 | rank1 |
+------------+-----------+-----------+
| ``csrow2`` | DIMM_A1 | DIMM_B1 |
- +------------+ | |
- | ``csrow3`` | | |
+ | | rank0 | rank0 |
+ +------------+ - | - |
+ | ``csrow3`` | rank1 | rank1 |
+------------+-----------+-----------+
In the above example, there are 4 physical slots on the motherboard
@@ -374,11 +379,13 @@ which the memory DIMM is placed. Thus, when 1 DIMM is placed in each
Channel, the csrows cross both DIMMs.
Memory DIMMs come single or dual "ranked". A rank is a populated csrow.
-Thus, 2 single ranked DIMMs, placed in slots DIMM_A0 and DIMM_B0 above
-will have just one csrow (csrow0). csrow1 will be empty. On the other
-hand, when 2 dual ranked DIMMs are similarly placed, then both csrow0
-and csrow1 will be populated. The pattern repeats itself for csrow2 and
-csrow3.
+In the example above 2 dual ranked DIMMs are similarly placed. Thus,
+both csrow0 and csrow1 are populated. On the other hand, when 2 single
+ranked DIMMs are placed in slots DIMM_A0 and DIMM_B0, then they will
+have just one csrow (csrow0) and csrow1 will be empty. The pattern
+repeats itself for csrow2 and csrow3. Also note that some memory
+controller doesn't have any logic to identify the memory module, see
+``rankX`` directories below.
The representation of the above is reflected in the directory
tree in EDAC's sysfs interface. Starting in directory
--
2.20.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