From: Kai Huang <kai.huang@intel.com>
To: Isaku Yamahata <isaku.yamahata@gmail.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
dave.hansen@intel.com, seanjc@google.com, pbonzini@redhat.com,
kirill.shutemov@linux.intel.com,
sathyanarayanan.kuppuswamy@linux.intel.com, peterz@infradead.org,
tony.luck@intel.com, ak@linux.intel.com,
dan.j.williams@intel.com, isaku.yamahata@intel.com
Subject: Re: [PATCH v2 09/21] x86/virt/tdx: Get information about TDX module and convertible memory
Date: Mon, 28 Mar 2022 14:30:05 +1300 [thread overview]
Message-ID: <f211441a6d23321e22517684159e2c28c8492b86.camel@intel.com> (raw)
In-Reply-To: <20220324174301.GA1212881@ls.amr.corp.intel.com>
> > +
> > +static int sanitize_cmrs(struct cmr_info *cmr_array, int cmr_num)
> > +{
> > + int i, j;
> > +
> > + /*
> > + * Intel TDX module spec, 20.7.3 CMR_INFO:
> > + *
> > + * TDH.SYS.INFO leaf function returns a MAX_CMRS (32) entry
> > + * array of CMR_INFO entries. The CMRs are sorted from the
> > + * lowest base address to the highest base address, and they
> > + * are non-overlapping.
> > + *
> > + * This implies that BIOS may generate invalid empty entries
> > + * if total CMRs are less than 32. Skip them manually.
> > + */
> > + for (i = 0; i < cmr_num; i++) {
> > + struct cmr_info *cmr = &cmr_array[i];
> > + struct cmr_info *prev_cmr = NULL;
> > +
> > + /* Skip further invalid CMRs */
> > + if (!cmr_valid(cmr))
> > + break;
> > +
> > + if (i > 0)
> > + prev_cmr = &cmr_array[i - 1];
> > +
> > + /*
> > + * It is a TDX firmware bug if CMRs are not
> > + * in address ascending order.
> > + */
> > + if (prev_cmr && ((prev_cmr->base + prev_cmr->size) >
> > + cmr->base)) {
> > + pr_err("Firmware bug: CMRs not in address ascending order.\n");
> > + return -EFAULT;
> > + }
> > + }
> > +
> > + /*
> > + * Also a sane BIOS should never generate invalid CMR(s) between
> > + * two valid CMRs. Sanity check this and simply return error in
> > + * this case.
> > + */
> > + for (j = i; j < cmr_num; j++)
> > + if (cmr_valid(&cmr_array[j])) {
> > + pr_err("Firmware bug: invalid CMR(s) among valid CMRs.\n");
> > + return -EFAULT;
> > + }
>
> This check doesn't make sense because above i-for loop has break.
The break in above i-for loop will hit at the first invalid CMR entry. Yes "j =
i" will make double check on this invalid CMR entry, but it should have no
problem. Or we can change to "j = i + 1" to skip the first invalid CMR entry.
Does this make sense?
>
> > +
> > + /*
> > + * Trim all tail invalid empty CMRs. BIOS should generate at
> > + * least one valid CMR, otherwise it's a TDX firmware bug.
> > + */
> > + tdx_cmr_num = i;
> > + if (!tdx_cmr_num) {
> > + pr_err("Firmware bug: No valid CMR.\n");
> > + return -EFAULT;
> > + }
>
> Something strange.
> Probably we'd like to check it by decrementing.
> for (i = cmr_num; i >= 0; i--)
> if (!cmr_valid()) // if last invalid cmr
> tdx_cmr_num
> // more check. overlapping
>
I don't know how does this look strange to you? As replied above, "i" is the
index to the first invalid CMR entry (or cmr_num, which is array size), so
"tdx_cmr_num = i" initializes the total valid CMR number, correct?
next prev parent reply other threads:[~2022-03-28 1:30 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-13 10:49 [PATCH v2 00/21] TDX host kernel support Kai Huang
2022-03-13 10:49 ` [PATCH v2 01/21] x86/virt/tdx: Detect SEAM Kai Huang
2022-03-23 3:21 ` Tian, Kevin
2022-03-28 3:55 ` Kai Huang
2022-03-28 8:10 ` Tian, Kevin
2022-03-29 17:52 ` Isaku Yamahata
2022-03-29 23:28 ` Kai Huang
2022-03-13 10:49 ` [PATCH v2 02/21] x86/virt/tdx: Detect TDX private KeyIDs Kai Huang
2022-03-13 10:49 ` [PATCH v2 03/21] x86/virt/tdx: Implement the SEAMCALL base function Kai Huang
2022-03-23 3:35 ` Tian, Kevin
2022-03-28 1:41 ` Kai Huang
2022-03-28 8:16 ` Tian, Kevin
2022-03-28 9:10 ` Kai Huang
2022-03-13 10:49 ` [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing TDX on demand Kai Huang
2022-03-23 6:49 ` Tian, Kevin
2022-03-28 1:57 ` Kai Huang
2022-03-28 8:26 ` Tian, Kevin
2022-03-28 9:24 ` Kai Huang
2022-03-28 11:47 ` Tian, Kevin
2022-03-28 22:55 ` Kai Huang
2022-03-29 2:36 ` Tian, Kevin
2022-03-29 3:10 ` Kai Huang
2022-03-29 3:17 ` Kai Huang
2022-03-13 10:49 ` [PATCH v2 05/21] x86/virt/tdx: Detect P-SEAMLDR and TDX module Kai Huang
2022-03-13 10:49 ` [PATCH v2 06/21] x86/virt/tdx: Shut down TDX module in case of error Kai Huang
2022-03-13 10:49 ` [PATCH v2 07/21] x86/virt/tdx: Do TDX module global initialization Kai Huang
2022-03-13 10:49 ` [PATCH v2 08/21] x86/virt/tdx: Do logical-cpu scope TDX module initialization Kai Huang
2022-03-13 10:49 ` [PATCH v2 09/21] x86/virt/tdx: Get information about TDX module and convertible memory Kai Huang
2022-03-24 17:43 ` Isaku Yamahata
2022-03-28 1:30 ` Kai Huang [this message]
2022-03-28 20:22 ` Isaku Yamahata
2022-03-28 20:30 ` Dave Hansen
2022-03-28 23:40 ` Kai Huang
2022-03-28 23:44 ` Dave Hansen
2022-03-29 0:04 ` Kai Huang
2022-03-13 10:49 ` [PATCH v2 10/21] x86/virt/tdx: Add placeholder to coveret all system RAM as TDX memory Kai Huang
2022-03-13 10:49 ` [PATCH v2 11/21] x86/virt/tdx: Choose to use " Kai Huang
2022-03-13 10:49 ` [PATCH v2 12/21] x86/virt/tdx: Create TDMRs to cover all system RAM Kai Huang
2022-03-13 10:49 ` [PATCH v2 13/21] x86/virt/tdx: Allocate and set up PAMTs for TDMRs Kai Huang
2022-03-24 18:06 ` Isaku Yamahata
2022-03-28 1:16 ` Kai Huang
2022-03-13 10:49 ` [PATCH v2 14/21] x86/virt/tdx: Set up reserved areas for all TDMRs Kai Huang
2022-03-13 10:49 ` [PATCH v2 15/21] x86/virt/tdx: Reserve TDX module global KeyID Kai Huang
2022-03-13 10:49 ` [PATCH v2 16/21] x86/virt/tdx: Configure TDX module with TDMRs and " Kai Huang
2022-03-13 10:49 ` [PATCH v2 17/21] x86/virt/tdx: Configure global KeyID on all packages Kai Huang
2022-03-24 18:18 ` isaku.yamahata
2022-03-28 1:19 ` Kai Huang
2022-03-13 10:49 ` [PATCH v2 18/21] x86/virt/tdx: Initialize all TDMRs Kai Huang
2022-03-13 10:49 ` [PATCH v2 19/21] x86: Flush cache of TDX private memory during kexec() Kai Huang
2022-03-13 10:50 ` [PATCH v2 20/21] x86/virt/tdx: Add kernel command line to opt-in TDX host support Kai Huang
2022-03-13 10:50 ` [PATCH v2 21/21] Documentation/x86: Add documentation for " Kai Huang
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=f211441a6d23321e22517684159e2c28c8492b86.camel@intel.com \
--to=kai.huang@intel.com \
--cc=ak@linux.intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@intel.com \
--cc=isaku.yamahata@gmail.com \
--cc=isaku.yamahata@intel.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=seanjc@google.com \
--cc=tony.luck@intel.com \
/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;
as well as URLs for NNTP newsgroup(s).