Re: [Qemu-devel] [PATCH v6 08/14] hmat acpi: Build Memory Side Cache Information Structure(s)

[Tao Xu]

On 7/9/2019 4:24 PM, Jonathan Cameron wrote:
> On Sun, 7 Jul 2019 22:29:52 +0800
> Tao Xu <address@hidden> wrote:
>
> > From: Liu Jingqi <address@hidden>
> >
> > This structure describes memory side cache information for memory
> > proximity domains if the memory side cache is present and the
> > physical device forms the memory side cache.
> > The software could use this information to effectively place
> > the data in memory to maximize the performance of the system
> > memory that use the memory side cache.
> >
> > Signed-off-by: Liu Jingqi <address@hidden>
> > Signed-off-by: Tao Xu <address@hidden>
>
> There is what I'll call a paste 'splurge' inline.
>
> Otherwise looks good.
>
> Reviewed-by: Jonathan Cameron <address@hidden>
>
> > ---
> >
> > Changes in v6:
> >      - Add descriptions from ACPI 6.3 spec
> > ---
> >   hw/acpi/hmat.c          | 64 ++++++++++++++++++++++++++++++++++++++++-
> >   hw/acpi/hmat.h          | 20 +++++++++++++
> >   include/qemu/typedefs.h |  1 +
> >   include/sysemu/numa.h   |  3 ++
> >   include/sysemu/sysemu.h |  2 ++
> >   5 files changed, 89 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
> > index 6dd39b0c85..a207581f11 100644
> > --- a/hw/acpi/hmat.c
> > +++ b/hw/acpi/hmat.c
> > @@ -133,14 +133,63 @@ static void build_hmat_lb(GArray *table_data, 
> > HMAT_LB_Info *numa_hmat_lb,
> >       }
> >   }
> >   
> > +/* ACPI 6.3: 5.2.27.5 Memory Side Cache Information Structure: Table 5-143 */
> > +static void build_hmat_cache(GArray *table_data, HMAT_Cache_Info *hmat_cache)
> > +{
> > +    /*
> > +     * Cache Attributes: Bits [3:0] ?C Total Cache Levels
> > +     * for this Memory Proximity Domain
> > +     */
> > +    uint32_t cache_attr = hmat_cache->total_levels & 0xF;
> > +
> > +    /* Bits [7:4] : Cache Level described in this structure */
> > +    cache_attr |= (hmat_cache->level & 0xF) << 4;
> > +
> > +    /* Bits [11:8] - Cache Associativity */
> > +    cache_attr |= (hmat_cache->associativity & 0xF) << 8;
> > +
> > +    /* Bits [15:12] - Write Policy */
> > +    cache_attr |= (hmat_cache->write_policy & 0xF) << 12;
> > +
> > +    /* Bits [31:16] - Cache Line size in bytes */
> > +    cache_attr |= (hmat_cache->line_size & 0xFFFF) << 16;
> > +
> > +    cache_attr = cpu_to_le32(cache_attr);
> > +
> > +    /* Type */
> > +    build_append_int_noprefix(table_data, 2, 2);
> > +    /* Reserved */
> > +    build_append_int_noprefix(table_data, 0, 2);
> > +    /* Length */
> > +    build_append_int_noprefix(table_data, 32, 4);
> > +    /* Proximity Domain for the Memory */
> > +    build_append_int_noprefix(table_data, hmat_cache->mem_proximity, 4);
> > +    /* Reserved */
> > +    build_append_int_noprefix(table_data, 0, 4);
> > +    /* Memory Side Cache Size */
> > +    build_append_int_noprefix(table_data, hmat_cache->size, 8);
> > +    /* Cache Attributes */
> > +    build_append_int_noprefix(table_data, cache_attr, 4);
> > +    /* Reserved */
> > +    build_append_int_noprefix(table_data, 0, 2);
> > +    /*
> > +     * Number of SMBIOS handles (n)
> > +     * Linux kernel uses Memory Side Cache Information Structure
> > +     * without SMBIOS entries for now, so set Number of SMBIOS handles
> > +     * as 0.
> > +     */
> > +    build_append_int_noprefix(table_data, 0, 2);
> > +}
> > +
> >   /* Build HMAT sub table structures */
> >   static void hmat_build_table_structs(GArray *table_data, NumaState *nstat)
> >   {
> >       uint16_t flags;
> >       uint32_t num_initiator = 0;
> >       uint32_t initiator_pxm[MAX_NODES];
> > -    int i, hrchy, type;
> > +    int i, hrchy, type, level;
> >       HMAT_LB_Info *numa_hmat_lb;
> > +    HMAT_Cache_Info *numa_hmat_cache;
> >   
> >       for (i = 0; i < nstat->num_nodes; i++) {
> >           flags = 0;
> > @@ -174,6 +223,19 @@ static void hmat_build_table_structs(GArray *table_data, 
> > NumaState *nstat)
> >               }
> >           }
> >       }
> > +
> > +    /*
> > +     * ACPI 6.3: 5.2.27.5 Memory Side Cache Information Structure:
> > +     * Table 5-143
> > +     */
> > +    for (i = 0; i < nstat->num_nodes; i++) {
> > +        for (level = 0; level <= MAX_HMAT_CACHE_LEVEL; level++) {
> > +            numa_hmat_cache = nstat->hmat_cache[i][level];
> > +            if (numa_hmat_cache) {
> > +                build_hmat_cache(table_data, numa_hmat_cache);
> > +            }
> > +        }
> > +    }
> >   }
> >   
> >   void build_hmat(GArray *table_data, BIOSLinker *linker, NumaState *nstat)
> > diff --git a/hw/acpi/hmat.h b/hw/acpi/hmat.h
> > index 9d5f407b8a..ba655281cc 100644
> > --- a/hw/acpi/hmat.h
> > +++ b/hw/acpi/hmat.h
> > @@ -79,6 +79,26 @@ struct HMAT_LB_Info {
> >       uint16_t    bandwidth[MAX_NODES][MAX_NODES];
> >   };
> >   
> > +struct HMAT_Cache_Info {
> > +    /* The memory proximity domain to which the memory belongs. */
> > +    uint32_t    mem_proximity;
> > +    /* Size of memory side cache in bytes. */
> > +    uint64_t    size;
> > +    /*
> > +     * Total cache levels for this memory
> > +     * pr#include "hw/acpi/aml-build.h"oximity domain.
>
> :)  Happens to us all. (I went back and checked original email in case
> I'd just inserted that.

Thank you for pointing it. Maybe I make a mistake here when I rebase it.
I will correct it.

> > +     */
> > +    uint8_t     total_levels;
> > +    /* Cache level described in this structure. */
> > +    uint8_t     level;
> > +    /* Cache Associativity: None/Direct Mapped/Comple Cache Indexing */
> > +    uint8_t     associativity;
> > +    /* Write Policy: None/Write Back(WB)/Write Through(WT) */
> > +    uint8_t     write_policy;
> > +    /* Cache Line size in bytes. */
> > +    uint16_t    line_size;
> > +};
> > +
> >   void build_hmat(GArray *table_data, BIOSLinker *linker, NumaState *nstat);
> >   
> >   #endif
> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> > index c0257e936b..d971f5109e 100644
> > --- a/include/qemu/typedefs.h
> > +++ b/include/qemu/typedefs.h
> > @@ -33,6 +33,7 @@ typedef struct FWCfgEntry FWCfgEntry;
> >   typedef struct FWCfgIoState FWCfgIoState;
> >   typedef struct FWCfgMemState FWCfgMemState;
> >   typedef struct FWCfgState FWCfgState;
> > +typedef struct HMAT_Cache_Info HMAT_Cache_Info;
> >   typedef struct HMAT_LB_Info HMAT_LB_Info;
> >   typedef struct HVFX86EmulatorState HVFX86EmulatorState;
> >   typedef struct I2CBus I2CBus;
> > diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
> > index 0b80bc2fa2..2e5bdcbd19 100644
> > --- a/include/sysemu/numa.h
> > +++ b/include/sysemu/numa.h
> > @@ -35,6 +35,9 @@ struct NumaState {
> >   
> >       /* NUMA modes HMAT Locality Latency and Bandwidth Information */
> >       HMAT_LB_Info *hmat_lb[HMAT_LB_LEVELS][HMAT_LB_TYPES];
> > +
> > +    /* Memory Side Cache Information Structure */
> > +    HMAT_Cache_Info *hmat_cache[MAX_NODES][MAX_HMAT_CACHE_LEVEL + 1];
> >   };
> >   typedef struct NumaState NumaState;
> >   
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index 3f83fc0d58..85c584c531 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -143,6 +143,8 @@ enum {
> >       HMAT_LB_DATA_WRITE_BANDWIDTH  = 5,
> >   };
> >   
> > +#define MAX_HMAT_CACHE_LEVEL        3
> > +
> >   #define HMAT_LB_LEVELS    (HMAT_LB_MEM_CACHE_3RD_LEVEL + 1)
> >   #define HMAT_LB_TYPES     (HMAT_LB_DATA_WRITE_BANDWIDTH + 1)
> >