Re: [PATCH] EDAC, ghes: Fix grain calculation

[Robert Richter <rrichter@marvell.com>]

On 14.06.19 00:41:30, Borislav Petkov wrote:
> On Thu, Jun 13, 2019 at 09:07:38PM +0000, Robert Richter wrote:
> >  grain_bits = fls_long(e->grain) + 1;
> 
> For 4K grain this makes grain_bits == 12.

fls(0) == 0
fls(1) == 1
fls(2) == 2
fls(4) == 3
...
fls(0x1000) == 13

grain_bits = fls_long(0x1000) + 1 = 14

> 
> > In ras_event.h the grain then is calculated different to edac_mc
> > again:
> > 
> >  1 << __entry->grain_bits
> 
> and this makes it 4096.

It then calculates to 16k.

> 
> Looks ok to me.
> 
> But if in James' example, physical_address_mask is 0xfffffffffffff000,
> doing
> 
>   grain_bits = fls_long(~mem_err->physical_addr_mask);
> 
> would make it 11 because ~mem_err->physical_addr_mask (negated) should
> be 0xfff.

If it is not power of 2 (e.g. 0xfff), with

 fls_long(e->grain) + 1;

from edac_mc there is still an off by one (so James' calculation for
ghes is correct).

Correct for edac_mc is:

 grain_bits = fls_long(e->grain ? e->grain - 1 : 0);

This works for both, power of 2 and less:

 grain_bits(0x1000) == 12;
 grain_bits(0x0fff) == 12;
 grain_bits(0x1001) == 13;
 grain_bits(0) == 0
 grain_bits(1) == 0
 grain_bits(2) == 1

James' calculation for ghes is correct in this particular case
(assuming a bit mask with (power of 2 - 1)).

But my formula is more generic.

-Robert