[SeaBIOS] [RFC PATCH] fw/pci: Add support for mapping Intel IGD OpRegion via QEMU

2016-02-01 Thread Alex Williamson
The proposed IGD OpRegion support in QEMU via vfio maps the host
OpRegion into VM system memory at the address written to the ASL
Storage register (0xFC).  The OpRegion contains a 16-byte signature
followed by a 4-byte size field.  Therefore SeaBIOS can allocate a
buffer of the typical size (8KB), this results in a matching e820
reserved entry for the range, then write the buffer address to the ASL
Storage register, blinking the OpRegion into the VM address space.  At
that point SeaBIOS can validate the signature and size and remap if
necessary.  If the OpRegion is larger than 8KB, this would result in
any conflicting ranges being temporarily mapped with the OpRegion,
which probably needs further discussion on what that could break.
Other options might be to use the same algorithm with an obscenely
sized initial buffer to make sure we do not overlap, always
re-allocating the proper sized buffer, or perhaps we could pass the
OpRegion itself or information about the OpRegion through fw_cfg.

With the posted kernel series[1] and QEMU series[2] (on top of Gerd's
IGD patches[3]), this makes the OpRegion available to the VM and
tracing shows that the guest driver does access it.

[1] https://lkml.org/lkml/2016/2/1/884
[2] https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg00202.html
[3] https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg00244.html

Signed-off-by: Alex Williamson 
---
 src/fw/pciinit.c |   50 ++
 1 file changed, 50 insertions(+)

diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index c31c2fa..4f3251e 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -257,6 +257,52 @@ static void ich9_smbus_setup(struct pci_device *dev, void 
*arg)
 pci_config_writeb(bdf, ICH9_SMB_HOSTC, ICH9_SMB_HOSTC_HST_EN);
 }
 
+static void intel_igd_opregion_setup(struct pci_device *dev, void *arg)
+{
+u16 bdf = dev->bdf;
+u32 orig;
+void *opregion;
+int size = 8;
+
+if (!CONFIG_QEMU)
+return;
+
+orig = pci_config_readl(bdf, 0xFC);
+
+realloc:
+opregion = malloc_high(size * 1024);
+if (!opregion) {
+warn_noalloc();
+return;
+}
+
+/*
+ * QEMU maps the OpRegion into system memory at the address written here,
+ * this overlaps our malloc, which marks the range e820 reserved.
+ */
+pci_config_writel(bdf, 0xFC, cpu_to_le32((u32)opregion));
+
+if (memcmp(opregion, "IntelGraphicsMem", 16)) {
+pci_config_writel(bdf, 0xFC, orig);
+free(opregion);
+return; /* the opregion didn't magically appear, not supported */
+}
+
+if (size == le32_to_cpu(*(u32 *)(opregion + 16))) {
+dprintf(1, "Intel IGD OpRegion enabled on %02x:%02x.%x\n",
+pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), pci_bdf_to_fn(bdf));
+return; /* success! */
+}
+
+pci_config_writel(bdf, 0xFC, orig);
+free(opregion);
+
+if (size == 8) { /* try once more with a new size */
+size = le32_to_cpu(*(u32 *)(opregion + 16));
+goto realloc;
+}
+}
+
 static const struct pci_device_id pci_device_tbl[] = {
 /* PIIX3/PIIX4 PCI to ISA bridge */
 PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371SB_0,
@@ -290,6 +336,10 @@ static const struct pci_device_id pci_device_tbl[] = {
 PCI_DEVICE_CLASS(PCI_VENDOR_ID_APPLE, 0x0017, 0xff00, apple_macio_setup),
 PCI_DEVICE_CLASS(PCI_VENDOR_ID_APPLE, 0x0022, 0xff00, apple_macio_setup),
 
+/* Intel IGD OpRegion setup */
+PCI_DEVICE_CLASS(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA,
+ intel_igd_opregion_setup),
+
 PCI_DEVICE_END,
 };
 


___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH v2 10/10] tpm: Write logs in TPM 2 format

2016-02-01 Thread Stefan Berger

On 02/01/2016 05:05 PM, Kevin O'Connor wrote:

On Fri, Jan 22, 2016 at 05:47:20PM -0500, Stefan Berger wrote:

From: Stefan Berger 

Introduce a log_entry 'object' that we use to hold the data to be logged in
one log entry. Pass this object around rather than the TPM 1.2 specific 'pcpes'
structure.

Write this log_entry in the format that is appropriate for the version of the
host's TPM. For TPM 1.2 write it in the 'pcpes' structure's format, for TPM 2
in the new TPM 2 format.

By using this method we can keep the API interface on systems with a TPM 2 even
though applications pass in the 'pcpes' structures directly. The log will still 
be
written in the appropriate format.

The TPM 2 log contains a TPM 1.2 type of entry of event type EV_NO_ACTION and
entry of type TCG_EfiSpeIdEventStruct as the first entry. This is described
in the EFI specification (section 5.3):

TCG EFI Protocol Specification for TPM Family 2.0

http://www.trustedcomputinggroup.org/resources/tcg_efi_protocol_specification


Signed-off-by: Stefan Berger 
---
  src/std/tcg.h |  35 
  src/tcgbios.c | 173 +-
  2 files changed, 183 insertions(+), 25 deletions(-)

diff --git a/src/std/tcg.h b/src/std/tcg.h
index 8466b14..1e8b250 100644
--- a/src/std/tcg.h
+++ b/src/std/tcg.h
@@ -89,6 +89,7 @@ enum irq_ids {
  
  /* event types: 10.4.1 / table 11 */

  #define EV_POST_CODE 1
+#define EV_NO_ACTION 3
  #define EV_SEPARATOR 4
  #define EV_ACTION5
  #define EV_EVENT_TAG 6
@@ -472,4 +473,38 @@ struct tpm2_req_hierarchycontrol {
  u8 state;
  } PACKED;
  
+/* TPM 2 log entry */

+
+struct tpml_digest_values_sha1 {
+u16 hashtype;
+u8 sha1[SHA1_BUFSIZE];
+};
+
+struct tcg_pcr_event2_sha1 {
+u32 pcrindex;
+u32 eventtype;
+u32 count; /* number of digests */
+struct tpml_digest_values_sha1 digests[1];
+u32 eventsize;
+u8 event[0];
+} PACKED;
+
+struct TCG_EfiSpecIdEventStruct {
+u8 signature[16];
+u32 platformClass;
+u8 specVersionMinor;
+u8 specVersionMajor;
+u8 specErrata;
+u8 uintnSize;
+u32 numberOfAlgorithms;
+struct TCG_EfiSpecIdEventAlgorithmSize {
+u16 algorithmId;
+u16 digestSize;
+} digestSizes[1];
+u8 vendorInfoSize;
+u8 vendorInfo[0];
+};
+
+#define TPM_TCPA_ACPI_CLASS_CLIENT 0
+
  #endif // tcg.h
diff --git a/src/tcgbios.c b/src/tcgbios.c
index f5500b1..369736c 100644
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
@@ -52,6 +52,71 @@ static const u8 TPM2_SelfTest_YES[] =  { TPM2_YES }; /* full 
test */
  
  typedef u8 tpm_ppi_code;
  
+

+static TPMVersion TPM_version;
+
+/
+ * Log entry 'object'
+ /

This section looks like it could be merged into the "ACPI TCPA table
interface" section as both sections are focused solely on logging.


+struct log_entry {
+u32 pcrindex;
+u32 eventtype;
+/* we only support SHA1 for TPM 2 */
+u8 sha1[SHA1_BUFSIZE];
+u32 eventdatasize;
+const void *event; /* TPM 1.2 & 2 */
+};

Instead of introducing 'struct log_entry' to be the superset of
tcg_pcr_event2_sha1 and pcpes, why not use tcg_pcr_event2_sha1
everywhere?  That is, replace all the 'struct pcpes *' with 'struct
tcg_pcr_event2_sha1 *' and then enhance tpm_log_event() to translate
to a pcpes for older logs.


Sure, can do that also.



Of course, the above is assuming the log should be in an "efi" format
- see previous email on that.



The format doesn't depend on the kind of firmware (efi vs. bios), it's 
more a TPM 2 type of log that supports different types of hashes per 
measurement.





[...]

+void tpm_log_init(void)
+{
+struct TCG_EfiSpecIdEventStruct event = {
+.signature = "Spec ID Event03",
+.platformClass = TPM_TCPA_ACPI_CLASS_CLIENT,
+.specVersionMinor = 0,
+.specVersionMajor = 2,
+.specErrata = 0,
+.uintnSize = 2,
+.numberOfAlgorithms = 1,
+.digestSizes[0] = {
+ .algorithmId = TPM2_ALG_SHA1,
+ .digestSize = SHA1_BUFSIZE,
+},
+.vendorInfoSize = 0,
+};
+struct log_entry le = {
+.eventtype = EV_NO_ACTION,
+.event = &event,
+};
+
+switch (TPM_version) {
+case TPM_VERSION_NONE:
+case TPM_VERSION_1_2:
+break;
+case TPM_VERSION_2:
+/* write a 1.2 type of entry */
+TPM_version = 1;
+tpm_log_event(&le);
+TPM_version = 2;

Altering the TPM_version seems ugly - I think tpm_log_event() could
take the version as a parameter to avoid that.


:-)

   Stefan


___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH v2 03/10] tpm: Prepare code for TPM 2 functions

2016-02-01 Thread Stefan Berger
"Kevin O'Connor"  wrote on 02/01/2016 04:54:53 PM:

> From: "Kevin O'Connor" 
> To: Stefan Berger/Watson/IBM@IBMUS
> Cc: seabios@seabios.org, Stefan Berger 
> Date: 02/01/2016 04:55 PM
> Subject: Re: [PATCH v2 03/10] tpm: Prepare code for TPM 2 functions
> 
> On Fri, Jan 22, 2016 at 05:47:13PM -0500, Stefan Berger wrote:
> > From: Stefan Berger 
> > 
> > This patch prepares the tcgbios.c file for extension with TPM 2
> > specific code by:
> > 
> >  o prefixing all TPM 1.2 specific functions with tpm12_
> >  o where necessary, introduce switch statements in tpm_ - 
prefixedfunctions;
> >here we branch into TPM versions specific code
> >  o introduce tpm_ - prefixed functions where necessary; mostly in 
those
> >cases where tpm12_ functions are too large and where the tpm_ 
function
> >then only holds the switch statement
> >  o leave FIXMEs where we need to write TPM 2 specific code; 
> subsequent patches
> >will replace those FIXMEs
> > 
> > Signed-off-by: Stefan Berger 
> > ---
> >  src/tcgbios.c | 311 
> +-
> >  1 file changed, 199 insertions(+), 112 deletions(-)
> > 
> > diff --git a/src/tcgbios.c b/src/tcgbios.c
> > index 799a8bf..7f314b7 100644
> > --- a/src/tcgbios.c
> > +++ b/src/tcgbios.c
> > @@ -171,7 +171,15 @@ tpm_is_working(void)
> >  int
> >  tpm_can_show_menu(void)
> >  {
> > -return tpm_is_working() && TPM_has_physical_presence;
> > +switch (TPM_version) {
> > +case TPM_VERSION_NONE:
> > +return 0;
> 
> I find these "case TPM_VERSION_NONE:" clauses to be a little
> confusing, both in this patches and the later patches, because I don't
> think any of these additional code paths are reachable.  I think it
> would be better to just have the branches that are active (ie, v1.2
> and v2).


I know. The 'enum' forced this. So I'll remove these then and make 
TPM_VERSION_NONE etc. individual #define's.

   Stefan


___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH] Fix comment typo

2016-02-01 Thread Kevin O'Connor
On Sat, Jan 30, 2016 at 03:50:38PM +0800, Cao jin wrote:
> Signed-off-by: Cao jin 

Thanks - I applied this patch.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH v2 10/10] tpm: Write logs in TPM 2 format

2016-02-01 Thread Kevin O'Connor
On Fri, Jan 22, 2016 at 05:47:20PM -0500, Stefan Berger wrote:
> From: Stefan Berger 
> 
> Introduce a log_entry 'object' that we use to hold the data to be logged in
> one log entry. Pass this object around rather than the TPM 1.2 specific 
> 'pcpes'
> structure.
> 
> Write this log_entry in the format that is appropriate for the version of the
> host's TPM. For TPM 1.2 write it in the 'pcpes' structure's format, for TPM 2
> in the new TPM 2 format.
> 
> By using this method we can keep the API interface on systems with a TPM 2 
> even
> though applications pass in the 'pcpes' structures directly. The log will 
> still be
> written in the appropriate format.
> 
> The TPM 2 log contains a TPM 1.2 type of entry of event type EV_NO_ACTION and
> entry of type TCG_EfiSpeIdEventStruct as the first entry. This is described
> in the EFI specification (section 5.3):
> 
> TCG EFI Protocol Specification for TPM Family 2.0
> 
> http://www.trustedcomputinggroup.org/resources/tcg_efi_protocol_specification
> 
> 
> Signed-off-by: Stefan Berger 
> ---
>  src/std/tcg.h |  35 
>  src/tcgbios.c | 173 
> +-
>  2 files changed, 183 insertions(+), 25 deletions(-)
> 
> diff --git a/src/std/tcg.h b/src/std/tcg.h
> index 8466b14..1e8b250 100644
> --- a/src/std/tcg.h
> +++ b/src/std/tcg.h
> @@ -89,6 +89,7 @@ enum irq_ids {
>  
>  /* event types: 10.4.1 / table 11 */
>  #define EV_POST_CODE 1
> +#define EV_NO_ACTION 3
>  #define EV_SEPARATOR 4
>  #define EV_ACTION5
>  #define EV_EVENT_TAG 6
> @@ -472,4 +473,38 @@ struct tpm2_req_hierarchycontrol {
>  u8 state;
>  } PACKED;
>  
> +/* TPM 2 log entry */
> +
> +struct tpml_digest_values_sha1 {
> +u16 hashtype;
> +u8 sha1[SHA1_BUFSIZE];
> +};
> +
> +struct tcg_pcr_event2_sha1 {
> +u32 pcrindex;
> +u32 eventtype;
> +u32 count; /* number of digests */
> +struct tpml_digest_values_sha1 digests[1];
> +u32 eventsize;
> +u8 event[0];
> +} PACKED;
> +
> +struct TCG_EfiSpecIdEventStruct {
> +u8 signature[16];
> +u32 platformClass;
> +u8 specVersionMinor;
> +u8 specVersionMajor;
> +u8 specErrata;
> +u8 uintnSize;
> +u32 numberOfAlgorithms;
> +struct TCG_EfiSpecIdEventAlgorithmSize {
> +u16 algorithmId;
> +u16 digestSize;
> +} digestSizes[1];
> +u8 vendorInfoSize;
> +u8 vendorInfo[0];
> +};
> +
> +#define TPM_TCPA_ACPI_CLASS_CLIENT 0
> +
>  #endif // tcg.h
> diff --git a/src/tcgbios.c b/src/tcgbios.c
> index f5500b1..369736c 100644
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -52,6 +52,71 @@ static const u8 TPM2_SelfTest_YES[] =  { TPM2_YES }; /* 
> full test */
>  
>  typedef u8 tpm_ppi_code;
>  
> +
> +static TPMVersion TPM_version;
> +
> +/
> + * Log entry 'object'
> + /

This section looks like it could be merged into the "ACPI TCPA table
interface" section as both sections are focused solely on logging.

> +struct log_entry {
> +u32 pcrindex;
> +u32 eventtype;
> +/* we only support SHA1 for TPM 2 */
> +u8 sha1[SHA1_BUFSIZE];
> +u32 eventdatasize;
> +const void *event; /* TPM 1.2 & 2 */
> +};

Instead of introducing 'struct log_entry' to be the superset of
tcg_pcr_event2_sha1 and pcpes, why not use tcg_pcr_event2_sha1
everywhere?  That is, replace all the 'struct pcpes *' with 'struct
tcg_pcr_event2_sha1 *' and then enhance tpm_log_event() to translate
to a pcpes for older logs.

Of course, the above is assuming the log should be in an "efi" format
- see previous email on that.

[...]
> +void tpm_log_init(void)
> +{
> +struct TCG_EfiSpecIdEventStruct event = {
> +.signature = "Spec ID Event03",
> +.platformClass = TPM_TCPA_ACPI_CLASS_CLIENT,
> +.specVersionMinor = 0,
> +.specVersionMajor = 2,
> +.specErrata = 0,
> +.uintnSize = 2,
> +.numberOfAlgorithms = 1,
> +.digestSizes[0] = {
> + .algorithmId = TPM2_ALG_SHA1,
> + .digestSize = SHA1_BUFSIZE,
> +},
> +.vendorInfoSize = 0,
> +};
> +struct log_entry le = {
> +.eventtype = EV_NO_ACTION,
> +.event = &event,
> +};
> +
> +switch (TPM_version) {
> +case TPM_VERSION_NONE:
> +case TPM_VERSION_1_2:
> +break;
> +case TPM_VERSION_2:
> +/* write a 1.2 type of entry */
> +TPM_version = 1;
> +tpm_log_event(&le);
> +TPM_version = 2;

Altering the TPM_version seems ugly - I think tpm_log_event() could
take the version as a parameter to avoid that.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [PATCH v2 03/10] tpm: Prepare code for TPM 2 functions

2016-02-01 Thread Kevin O'Connor
On Fri, Jan 22, 2016 at 05:47:13PM -0500, Stefan Berger wrote:
> From: Stefan Berger 
> 
> This patch prepares the tcgbios.c file for extension with TPM 2
> specific code by:
> 
>  o prefixing all TPM 1.2 specific functions with tpm12_
>  o where necessary, introduce switch statements in tpm_ - prefixed functions;
>here we branch into TPM versions specific code
>  o introduce tpm_ - prefixed functions where necessary; mostly in those
>cases where tpm12_ functions are too large and where the tpm_ function
>then only holds the switch statement
>  o leave FIXMEs where we need to write TPM 2 specific code; subsequent patches
>will replace those FIXMEs
> 
> Signed-off-by: Stefan Berger 
> ---
>  src/tcgbios.c | 311 
> +-
>  1 file changed, 199 insertions(+), 112 deletions(-)
> 
> diff --git a/src/tcgbios.c b/src/tcgbios.c
> index 799a8bf..7f314b7 100644
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -171,7 +171,15 @@ tpm_is_working(void)
>  int
>  tpm_can_show_menu(void)
>  {
> -return tpm_is_working() && TPM_has_physical_presence;
> +switch (TPM_version) {
> +case TPM_VERSION_NONE:
> +return 0;

I find these "case TPM_VERSION_NONE:" clauses to be a little
confusing, both in this patches and the later patches, because I don't
think any of these additional code paths are reachable.  I think it
would be better to just have the branches that are active (ie, v1.2
and v2).

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


Re: [SeaBIOS] [RFC PATCH v1 0/9] Add TPM 2 support

2016-02-01 Thread Kevin O'Connor
On Fri, Jan 22, 2016 at 03:27:28PM -0500, Stefan Berger wrote:
> "Kevin O'Connor"  wrote on 01/21/2016 05:37:29 PM:
> > Thanks Stefan.  In general it looks good to me.  I have a few
> > comments, which I'll send separately.  All of my comments could be
> > addressed after committing this series if desired.
> 
> I can address those comments and repost a V2 with the 10th patch adding 
> the part for the logging.

Hi Stefan.  Sorry for the delay in responding.  I have a couple of
comments on the new patch series which I will respond with separately.

> > How does one test and/or use this support?  Does QEMU have support, or
> > is there hardware available on coreboot with the tpm2 hardware?
> 
> I did all the testing of these patches with the vTPM with CUSE interface 
> integrated into QEMU. Unfortunately the vTPM-QEMU integration train seems 
> a wreck now following comments on QEMU mailing list. So, I don't know of 
> any TPM 2 hardware out there, less so hardware where coreboot runs. So 
> that's probably currently the number one problem.

Normally, I prefer to wait until upstream has committed the equivalent
patches.  I think there is some leeway here, however, because this
series could be considered as adding support for additional hardware.

That said, if you don't know of any TPM2 hardware that is shipping, it
does raise the possibility that the specs might change by the time
actual hardware does ship.  What is your feel for the trade-off
between merging now and merging after actual implementations exist?

> You know the TPM 1.2 PC BIOS specification, right? I think we can say that 
> many of the functions implemented in this series for TPM 2 are necessary 
> because of how it's done for TPM 1.2 as well as properties of the TPM 2 
> device. This includes the TPM initialization, S3 support, setting of 
> timeouts, menu items, etc. The problem with TPM 2 is that there's no 
> official spec for TPM 2 for a BIOS. So it's not quite clear to me how much 
> leeway we have to go about this in the areas of ACPI tables for logging 
> and the API. Regarding these topics:
> 
> ACPI tables for logging: The (U)EFI specification for TPM 2 don't require 
> a TCPA table with the logging area because there seems to be an API for 
> the OS for retrieving the log. UEFI seems to log into just some buffer, 
> not connected to any ACPI table. For the BIOS we would still need that 
> TCPA table. QEMU currently provides that. The Linux kernel (and all other 
> OSes -- uuuh) would then have to allow a TCPA table for logging for TPM 2 
> even though we cannot point to a spec for that. Not sure whether we can 
> create a standard for this little gap here...

It sounds like the creators of the spec assumed only EFI machines
would have a TPM2 device.  Unless there is evidence that OSes will
accept the ACPI/TCPA table in the new format, I'd be inclined to leave
it in the old format.

> BIOS API: Some functions pass the entry to write into the log via the 
> function directly. Patch 10 handles that and transforms that entry into 
> the log entry format as required for TPM 1.2 or TPM 2 (log entries are 
> differently formatted for TPM 1.2 and for TPM 2). So the only remaining 
> problem I know of is the function that allows one to pass TPM commands 
> through to the TPM. This may end up causing problems in the application if 
> it was written for TPM 1.2 and now there's a TPM 2 running underneath, 
> which doesn't understand the TPM 1.2 commands. I would say this is likely 
> the smaller of the problems also considering that there are not many 
> applications out there that use that API call. Possibility to just shut 
> down that function call is certainly there.

I'd say returning an error code for pass-through command requests is
the safest solution.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios