linux-kernel - Re: [PATCH 2/3] ntb_hw_switchtec: Added support of >=4G memory windows

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] [day] [month] [year] [list]

Date:   Tue, 27 Nov 2018 11:06:16 -0500
From:   Jon Mason <jdmason@...zu.us>
To:     wesley.sheng@...rochip.com
Cc:     Kurt Schwemmer <kurt.schwemmer@...rosemi.com>,
        Logan Gunthorpe <logang@...tatee.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Allen Hubbe <allenbh@...il.com>, linux-pci@...r.kernel.org,
        linux-ntb@...glegroups.com,
        linux-kernel <linux-kernel@...r.kernel.org>,
        wesleyshenggit@...a.com
Subject: Re: [PATCH 2/3] ntb_hw_switchtec: Added support of >=4G memory windows

On Wed, Nov 21, 2018 at 9:19 PM Wesley Sheng <wesley.sheng@...rochip.com> wrote:
>
> From: Paul Selles <paul.selles@...rochip.com>
>
> Current Switchtec's BAR setup registers are limited to 32bits,
> corresponding to the maximum MW (memory window) size is <4G.
>
> Increase the MW sizes with the addition of the BAR Setup Extension
> Register for the upper 32bits of a 64bits MW size. This increases the MW
> range to between 4K and 2^63.
>
> Reported-by: Boris Glimcher <boris.glimcher@....com>
> Signed-off-by: Paul Selles <paul.selles@...rochip.com>
> Signed-off-by: Wesley Sheng <wesley.sheng@...rochip.com>
> ---
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 9 +++++++--
>  include/linux/switchtec.h              | 6 +++++-
>  2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> index 9916bc5..32850fb 100644
> --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
> @@ -264,6 +264,7 @@ static void switchtec_ntb_mw_clr_direct(struct switchtec_ntb *sndev, int idx)
>         ctl_val &= ~NTB_CTRL_BAR_DIR_WIN_EN;
>         iowrite32(ctl_val, &ctl->bar_entry[bar].ctl);
>         iowrite32(0, &ctl->bar_entry[bar].win_size);
> +       iowrite32(0, &ctl->bar_ext_entry[bar].win_size);
>         iowrite64(sndev->self_partition, &ctl->bar_entry[bar].xlate_addr);
>  }
>
> @@ -286,7 +287,9 @@ static void switchtec_ntb_mw_set_direct(struct switchtec_ntb *sndev, int idx,
>         ctl_val |= NTB_CTRL_BAR_DIR_WIN_EN;
>
>         iowrite32(ctl_val, &ctl->bar_entry[bar].ctl);
> -       iowrite32(xlate_pos | size, &ctl->bar_entry[bar].win_size);
> +       iowrite32(xlate_pos | (size & 0xFFFFF000),
> +                 &ctl->bar_entry[bar].win_size);
> +       iowrite32(size >> 32, &ctl->bar_ext_entry[bar].win_size);

Thanks for the patches.  Overall the look good.  Per the kbuild email,
size_t is 32bits on 32bit arch.  So, this is going to have compile
warnings on those.  Please address this and resubmit.

Also, patches 1 and 3 are bug fixes.  Please do the following, reorder
the patches to make the bug fixes first and add a "Fixes" line to the
commit messages (see
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes).
This will allow me to split up the series and get the bug fixes into
v4.20 (and the stable trees).

Thanks,
Jon

>         iowrite64(sndev->self_partition | addr,
>                   &ctl->bar_entry[bar].xlate_addr);
>  }
> @@ -1053,7 +1056,9 @@ static int crosslink_setup_mws(struct switchtec_ntb *sndev, int ntb_lut_idx,
>                 ctl_val |= NTB_CTRL_BAR_DIR_WIN_EN;
>
>                 iowrite32(ctl_val, &ctl->bar_entry[bar].ctl);
> -               iowrite32(xlate_pos | size, &ctl->bar_entry[bar].win_size);
> +               iowrite32(xlate_pos | (size & 0xFFFFF000),
> +                         &ctl->bar_entry[bar].win_size);
> +               iowrite32(size >> 32, &ctl->bar_ext_entry[bar].win_size);
>                 iowrite64(sndev->peer_partition | addr,
>                           &ctl->bar_entry[bar].xlate_addr);
>         }
> diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
> index eee0412..1e6e333 100644
> --- a/include/linux/switchtec.h
> +++ b/include/linux/switchtec.h
> @@ -248,7 +248,11 @@ struct ntb_ctrl_regs {
>                 u32 win_size;
>                 u64 xlate_addr;
>         } bar_entry[6];
> -       u32 reserved2[216];
> +       struct {
> +               u32 win_size;
> +               u32 reserved[3];
> +       } bar_ext_entry[6];
> +       u32 reserved2[192];
>         u32 req_id_table[256];
>         u32 reserved3[512];
>         u64 lut_entry[512];
> --
> 2.7.4
>