mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org
Cc: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	hpa@zytor.com, jon.grimm@amd.com,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Thomas Lendacky <Thomas.Lendacky@amd.com>
Subject: Re: [PATCH] x86/fpu: Warn only when CPU-provided sizes less than struct declaration
Date: Fri, 6 Dec 2019 07:28:58 -0800	[thread overview]
Message-ID: <4b20cff5-6e16-3599-4fc1-4f51d7c18d1d@intel.com> (raw)
In-Reply-To: <68bdd6f0-a229-433a-9234-303a3b02b092@amd.com>

On 12/6/19 12:14 AM, Suravee Suthikulpanit wrote:
> On 12/4/19 12:27 AM, Dave Hansen wrote:
>> On 12/3/19 1:01 AM, Suravee Suthikulpanit wrote:
>>> The current XCHECK_SZ macro warns if the XFEATURE size reported
>>> by CPUID does not match the size of kernel structure. However, depending
>>> on the hardware implementation, CPUID can report the XSAVE state size
>>> larger than the size of C structures defined for each of the XSAVE state
>>> due to padding.
>>
>> We have existing architecture for padding.  See xfeature_is_aligned(),
>> for instance.  Are you saying that there are implementations out there
>> that do padding which is not otherwise enumerated and that they do it
>> within the size of the enumerated stat
> Yes, the implementation includes the padding size within the size of
> the enumerated state. This results in the reported size larger than
> the amount needed by the feature.

I don't think we've ever had XSAVE state that differed in size between
implementations.  This kind of thing ensures that we can't have any
statically-defined inspection into the XSAVE state.

It also increases the amount of blind trust that we have in the CPU
implementations.  However, those warnings were specifically added at
Ingo's behest (IIRC) to reduce our blind trust in the CPU.

>>> Such case should be safe and should not need to generate warning
>>> message.
>>
>> I've seen these error messages trip before, but only on pre-production
>> processors with goofy microcode.  I'd be really suspicious that this is
>> just papering over a processor issue.  Or, that perhaps the compacted
>> form works but the standard form is broken somehow.
> 
> I have verified with the HW folks and the have confirmed that this is
> to be expected.

From a review perspective, I'd really appreciate being able to have a
concrete discussion about exactly which state this is and exactly how
much padding we are talking about and *why* the existing padding
architecture can't be used.  I'd also want guarantees about this state
not getting used for any real state, *ever*.

I'm not sure how we can have those conversations without more details,
though.  Would you be able, for instance, to have a private discussion
with the x86 maintainers?  That discussion could, understandably,
exclude folks from Intel.

  reply	other threads:[~2019-12-06 15:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03  9:01 Suravee Suthikulpanit
2019-12-03 10:34 ` Sebastian Andrzej Siewior
2019-12-06  8:14   ` Suravee Suthikulpanit
2019-12-03 17:27 ` Dave Hansen
2019-12-06  8:14   ` Suravee Suthikulpanit
2019-12-06 15:28     ` Dave Hansen [this message]
2019-12-11  5:24       ` Suravee Suthikulpanit
2019-12-11 14:13         ` Dave Hansen
2019-12-12  6:52           ` Suravee Suthikulpanit
2020-01-13  9:54             ` Suravee Suthikulpanit
2020-01-15 16:54               ` Dave Hansen
2020-04-29 19:18       ` AMD / Memory Protection Keys Dave Hansen
2020-05-01 23:50         ` Suravee Suthikulpanit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4b20cff5-6e16-3599-4fc1-4f51d7c18d1d@intel.com \
    --to=dave.hansen@intel.com \
    --cc=Thomas.Lendacky@amd.com \
    --cc=bigeasy@linutronix.de \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jon.grimm@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®