On Wed, 9 Oct 2019 14:39:46 +0800 Tao Xu <tao3...@intel.com> wrote: > On 10/2/2019 11:16 PM, Igor Mammedov wrote: > > On Fri, 20 Sep 2019 15:43:44 +0800 > > Tao Xu <tao3...@intel.com> wrote: > > > [...] > >> +struct HMAT_LB_Info { > >> + /* Indicates it's memory or the specified level memory side cache. */ > >> + uint8_t hierarchy; > >> + > >> + /* Present the type of data, access/read/write latency or bandwidth. > >> */ > >> + uint8_t data_type; > >> + > >> + /* Array to store the latencies */ > > specify units it's stored in > > > >> + uint64_t *latency; > >> + > >> + /* Array to store the bandwidthes */ > > ditto > > > >> + uint64_t *bandwidth; > > btw: > > > > what was the reason for picking uint64_t for storing above values? > > > > it seems in this patch you dumb down bandwidth to MB/s above but > > store latency as is. > > Because I want to store the bandwidth or latency value (minimum unit) > that user input. In HMAT, the minimum unit of bandwidth is MB/s, but in > QAPI, the minimum unit of size is Byte. So I convert size into MB/s and > time unit is "ps", need not convert. Just be consistent and store (user input) raw values for both fields (i.e. B/s PS/s) and post-process them later to uint16_t.
> > and then in 9/11 build_hmat_lb you divide that on 'base' units, > > where are guaranties that value stored here will fit into 2 bytes > > used in HMAT to store it in the table? > > > For HMAT spec, for a matrix of bandwidth or latency, there is only one > base (in order to save ACPI tables space). We need to extract base for a > matrix, but user input bandwidth or latency line by line. So after all > data input, we can extract the base (as in 9/11). > > There is another benefit. If user input different but similar units, > such as "10ns" and "100ps", we can also store them. Only If user input > big gap units, such as "1ps" and "1000ms". we can't store them and raise > error. No disagreement here, but I suggest to move verification and base calculation from 09/11 into a separate patch (right after this one) and doing it at numa_complete_configuration() time. To store calculated base you can add a common_base field to sub-table structure (HMAT_LB_Info) and use it when building ACPI table without extra calculations. > > if this structure should store values in terms on HMAT table it should > > probably use uint16_t and check that user provided value won't overflow > > at the time of CLI parsing. > > >