Re: [Xen-devel] [PATCH 15/15] xen: tools: expose EPC in ACPI table

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.07.17 at 10:16, <kaih.linux@xxxxxxxxx> wrote:
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -330,6 +330,15 @@ cpuid(uint32_t idx, uint32_t *eax, uint32_t *ebx, 
> uint32_t *ecx, uint32_t *edx)
>          : "0" (idx) );
>  }
>  
> +void cpuid_count(uint32_t idx, uint32_t count, uint32_t *eax,

Please name the first two leaf and subleaf.

> @@ -888,6 +897,18 @@ static uint8_t acpi_lapic_id(unsigned cpu)
>      return LAPIC_ID(cpu);
>  }
>  
> +static void get_epc_info(struct acpi_config *config)
> +{
> +    uint32_t eax, ebx, ecx, edx;
> +
> +    cpuid_count(0x12, 0x2, &eax, &ebx, &ecx, &edx);
> +
> +    config->epc_base = (((uint64_t)(ebx & 0xfffff)) << 32) |
> +                       (uint64_t)(eax & 0xfffff000);

Pointless cast.

> +    config->epc_size = (((uint64_t)(edx & 0xfffff)) << 32) |
> +                       (uint64_t)(ecx & 0xfffff000);

Again.

> --- a/tools/libacpi/dsdt.asl
> +++ b/tools/libacpi/dsdt.asl
> @@ -441,6 +441,55 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "Xen", "HVM", 0)
>                  }
>              }
>          }
> +
> +        Device (EPC)
> +        {
> +            Name (_HID, EisaId ("INT0E0C"))
> +            Name (_STR, Unicode ("Enclave Page Cache 1.5"))
> +            Name (_MLS, Package (0x01)
> +            {
> +                Package (0x02)
> +                {
> +                    "en",
> +                    Unicode ("Enclave Page Cache 1.5")
> +                }
> +            })
> +            Name (RBUF, ResourceTemplate ()
> +            {
> +                QWordMemory (ResourceConsumer, PosDecode, MinFixed, MaxFixed,
> +                    Cacheable, ReadWrite,
> +                    0x0000000000000000, // Granularity
> +                    0x0000000000000000, // Range Minimum
> +                    0x0000000000000000, // Range Maximum
> +                    0x0000000000000000, // Translation Offset
> +                    0x0000000000000001, // Length
> +                    ,, _Y03,
> +                    AddressRangeMemory, TypeStatic)
> +            })
> +
> +            Method(_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
> +            {
> +                CreateQwordField (RBUF, \_SB.EPC._Y03._MIN, EMIN) // _MIN: 
> Minimuum Base Address
> +                CreateQwordField (RBUF, \_SB.EPC._Y03._MAX, EMAX) // _MIN: 
> Maximum Base Address
> +                CreateQwordField (RBUF, \_SB.EPC._Y03._LEN, ELEN) // _LEN: 
> Length

Please see the comment in _SB.PCI0._CRS regarding operations
on qword fields. Even if we may not formally support the named
Windows versions anymore, we should continue to be careful
here. You could have noticed this by seeing that ...

> @@ -21,6 +21,8 @@
>             LMIN, 32,
>             HMIN, 32,
>             LLEN, 32,
> -           HLEN, 32
> +           HLEN, 32,
> +           EMIN, 64,
> +           ELEN, 64,
>         }

... there have been no 64-bit fields here so far.

> @@ -156,6 +156,9 @@ static int init_acpi_config(libxl__gc *gc,
>      config->lapic_id = acpi_lapic_id;
>      config->acpi_revision = 5;
>  
> +    config->epc_base = b_info->u.hvm.sgx.epcbase;
> +    config->epc_size = (b_info->u.hvm.sgx.epckb << 10);

Pointless parentheses. Plus I guess the field names could do with
an underscore separator in the middle - it took me a moment to
realize this is a kB value (explaining the shift by 10).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel