netdev - Re: [PATCH net-next 02/15] ice: cache NVM module bank information

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 29 Jan 2021 16:36:12 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Jacob Keller <jacob.e.keller@...el.com>
Cc:     Tony Nguyen <anthony.l.nguyen@...el.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        sassmann@...hat.com, Tony Brelinski <tonyx.brelinski@...el.com>
Subject: Re: [PATCH net-next 02/15] ice: cache NVM module bank information

On Fri, Jan 29, 2021 at 4:32 PM Jacob Keller <jacob.e.keller@...el.com> wrote:
>
>
>
> On 1/29/2021 1:04 PM, Willem de Bruijn wrote:
> > On Fri, Jan 29, 2021 at 4:01 PM Willem de Bruijn
> > <willemdebruijn.kernel@...il.com> wrote:
> >>
> >> On Thu, Jan 28, 2021 at 7:46 PM Tony Nguyen <anthony.l.nguyen@...el.com> wrote:
> >>>
> >>> From: Jacob Keller <jacob.e.keller@...el.com>
> >>>
> >>> The ice flash contains two copies of each of the NVM, Option ROM, and
> >>> Netlist modules. Each bank has a pointer word and a size word. In order
> >>> to correctly read from the active flash bank, the driver must calculate
> >>> the offset manually.
> >>>
> >>> During NVM initialization, read the Shadow RAM control word and
> >>> determine which bank is active for each NVM module. Additionally, cache
> >>> the size and pointer values for use in calculating the correct offset.
> >>>
> >>> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> >>> Tested-by: Tony Brelinski <tonyx.brelinski@...el.com>
> >>> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> >>> ---
> >>>  drivers/net/ethernet/intel/ice/ice_nvm.c  | 151 ++++++++++++++++++++++
> >>>  drivers/net/ethernet/intel/ice/ice_type.h |  37 ++++++
> >>>  2 files changed, 188 insertions(+)
> >>>
> >>> diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
> >>> index b0f0b4fc266b..308344045397 100644
> >>> --- a/drivers/net/ethernet/intel/ice/ice_nvm.c
> >>> +++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
> >>> @@ -603,6 +603,151 @@ static enum ice_status ice_discover_flash_size(struct ice_hw *hw)
> >>>         return status;
> >>>  }
> >>>
> >>> +/**
> >>> + * ice_read_sr_pointer - Read the value of a Shadow RAM pointer word
> >>> + * @hw: pointer to the HW structure
> >>> + * @offset: the word offset of the Shadow RAM word to read
> >>> + * @pointer: pointer value read from Shadow RAM
> >>> + *
> >>> + * Read the given Shadow RAM word, and convert it to a pointer value specified
> >>> + * in bytes. This function assumes the specified offset is a valid pointer
> >>> + * word.
> >>> + *
> >>> + * Each pointer word specifies whether it is stored in word size or 4KB
> >>> + * sector size by using the highest bit. The reported pointer value will be in
> >>> + * bytes, intended for flat NVM reads.
> >>> + */
> >>> +static enum ice_status
> >>> +ice_read_sr_pointer(struct ice_hw *hw, u16 offset, u32 *pointer)
> >>> +{
> >>> +       enum ice_status status;
> >>> +       u16 value;
> >>> +
> >>> +       status = ice_read_sr_word(hw, offset, &value);
> >>> +       if (status)
> >>> +               return status;
> >>> +
> >>> +       /* Determine if the pointer is in 4KB or word units */
> >>> +       if (value & ICE_SR_NVM_PTR_4KB_UNITS)
> >>> +               *pointer = (value & ~ICE_SR_NVM_PTR_4KB_UNITS) * 4 * 1024;
> >>> +       else
> >>> +               *pointer = value * 2;
> >>
> >> Should this be << 2, for 4B words?
> >
> > Never mind, sorry. I gather from patch 3 that wordsize is 16b.
> >
>
>
> Ah, yes that could have been explained a bit better. In this context, a
> word is indeed 2 bytes.
>
> Perhaps we could have used "<< 1" and "<< 12" or similar instead of the
> multiplication, but I felt this was a bit more clear.

Thanks. That doesn't matter (for me). I just wrongly assumed wordsize to be 4B.