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:   Tue, 15 Mar 2022 12:39:58 +0000
From:   Marc Mattmüller 
        <Marc.Mattmueller@...module.com>
To:     "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "avri.altman@....com" <avri.altman@....com>
CC:     "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] mmc: core: add reliable write setting to sysfs and
 update on read

On Tue, 2022-03-15 at 10:39 +0100, Ulf Hansson wrote:
> On Thu, 10 Mar 2022 at 15:08, Avri Altman <Avri.Altman@....com>
> wrote:
> > 
> > > The mmc reliable write setting (from ext_csd) was not available
> > > on the sysfs.
> > > Thus, added rel_param and rel_write_set to sysfs and added the
> > > update of
> > > rel_write_set on sysfs read.
> > Here also - why adding ABI when its already available via mmc-
> > utils?
> > 
> > Thanks,
> > Avri
> 
> FYI, I agree with Avri here. Please use mmc-utils.
> 
> In case the values in sysfs become confusing as they may be outdated
> at some point, perhaps we should consider dropping them?
> 
> Kind regards
> Uffe

Hi,
yes I was confused about having the lifetime values available in the 
sysfs but without having an update over time. The reliable write values
were some kind of logic for me to add them as well. That's why I was
hesitating first to create a single patch for all these values but as
it concerns two different topics I decided to create a patchset.

Nevertheless, if you propose to use the mmc-utils for the dynamic
values, it is less confusing if you just keep only the static
values and drop the dynamic ones.

Kind regards
Marc

> 
> > > Signed-off-by: Marc Mattmueller <marc.mattmueller@...module.com>
> > > ---
> > >  drivers/mmc/core/mmc.c   | 29 +++++++++++++++++++++++++++++
> > >  include/linux/mmc/card.h |  1 +
> > >  include/linux/mmc/mmc.h  |  1 +
> > >  3 files changed, 31 insertions(+)
> > > 
> > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > > index
> > > d9537c894e33..a64d1ecb0de9 100644
> > > --- a/drivers/mmc/core/mmc.c
> > > +++ b/drivers/mmc/core/mmc.c
> > > @@ -364,6 +364,10 @@ static int
> > > mmc_update_ext_csd_runtime_params(struct mmc_card *card, u8
> > > *ext_csd)  {
> > >         int err = 0;
> > > 
> > > +       /* eMMC v4.41 or later */
> > > +       if (card->ext_csd.rev >= 5)
> > > +               card->ext_csd.rel_wr_set =
> > > ext_csd[EXT_CSD_WR_REL_SET];
> > > +
> > >         /* eMMC v5 or later */
> > >         if (card->ext_csd.rev >= 7) {
> > >                 card->ext_csd.pre_eol_info =
> > > ext_csd[EXT_CSD_PRE_EOL_INFO];
> > > @@ -587,6 +591,7 @@ static int mmc_decode_ext_csd(struct mmc_card
> > > *card, u8 *ext_csd)
> > >                 }
> > > 
> > >                 card->ext_csd.rel_param =
> > > ext_csd[EXT_CSD_WR_REL_PARAM];
> > > +               card->ext_csd.rel_wr_set =
> > > ext_csd[EXT_CSD_WR_REL_SET];
> > >                 card->ext_csd.rst_n_function =
> > > ext_csd[EXT_CSD_RST_N_FUNCTION];
> > > 
> > >                 /*
> > > @@ -820,6 +825,7 @@ MMC_DEV_ATTR(name, "%s\n", card-
> > > > cid.prod_name);  MMC_DEV_ATTR(oemid, "0x%04x\n", card-
> > > > >cid.oemid);
> > > MMC_DEV_ATTR(prv, "0x%x\n", card->cid.prv);  MMC_DEV_ATTR(rev,
> > > "0x%x\n", card->ext_csd.rev);
> > > +MMC_DEV_ATTR(rel_param, "0x%02x\n", card->ext_csd.rel_param);
> > >  MMC_DEV_ATTR(serial, "0x%08x\n", card->cid.serial);
> > > MMC_DEV_ATTR(enhanced_area_offset, "%llu\n",
> > >                 card->ext_csd.enhanced_area_offset);
> > > @@ -886,6 +892,27 @@ static ssize_t pre_eol_info_show(struct
> > > device
> > > *dev,
> > > 
> > >  static DEVICE_ATTR_RO(pre_eol_info);
> > > 
> > > +static ssize_t rel_write_set_show(struct device *dev,
> > > +                                 struct device_attribute *attr,
> > > +                                 char *buf) {
> > > +       int err = 0;
> > > +       struct mmc_card *card = mmc_dev_to_card(dev);
> > > +
> > > +       /* before eMMC v4.41 */
> > > +       if (card->ext_csd.rev < 5)
> > > +               return sprintf(buf, "%s\n", "-");
> > > +
> > > +       /* eMMC v4.41 or later */
> > > +       err = mmc_update_csd(card);
> > > +       if (err)
> > > +               return (ssize_t)err;
> > > +
> > > +       return sprintf(buf, "0x%02x\n", card-
> > > >ext_csd.rel_wr_set); }
> > > +
> > > +static DEVICE_ATTR_RO(rel_write_set);
> > > +
> > >  static ssize_t mmc_fwrev_show(struct device *dev,
> > >                               struct device_attribute *attr,
> > >                               char *buf) @@ -931,6 +958,8 @@
> > > static struct attribute
> > > *mmc_std_attrs[] = {
> > >         &dev_attr_oemid.attr,
> > >         &dev_attr_prv.attr,
> > >         &dev_attr_rev.attr,
> > > +       &dev_attr_rel_param.attr,
> > > +       &dev_attr_rel_write_set.attr,
> > >         &dev_attr_pre_eol_info.attr,
> > >         &dev_attr_life_time.attr,
> > >         &dev_attr_serial.attr,
> > > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> > > index
> > > 37f975875102..21c47893fcb4 100644
> > > --- a/include/linux/mmc/card.h
> > > +++ b/include/linux/mmc/card.h
> > > @@ -48,6 +48,7 @@ struct mmc_ext_csd {
> > >         u8                      sec_feature_support;
> > >         u8                      rel_sectors;
> > >         u8                      rel_param;
> > > +       u8                      rel_wr_set;
> > >         bool                    enhanced_rpmb_supported;
> > >         u8                      part_config;
> > >         u8                      cache_ctrl;
> > > diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> > > index
> > > d9a65c6a8816..42afd442a70a 100644
> > > --- a/include/linux/mmc/mmc.h
> > > +++ b/include/linux/mmc/mmc.h
> > > @@ -266,6 +266,7 @@ static inline bool mmc_ready_for_data(u32
> > > status)
> > >  #define EXT_CSD_BKOPS_START            164     /* W */
> > >  #define EXT_CSD_SANITIZE_START         165     /* W */
> > >  #define EXT_CSD_WR_REL_PARAM           166     /* RO */
> > > +#define EXT_CSD_WR_REL_SET             167     /* R/W */
> > >  #define EXT_CSD_RPMB_MULT              168     /* RO */
> > >  #define EXT_CSD_FW_CONFIG              169     /* R/W */
> > >  #define EXT_CSD_BOOT_WP                        173     /* R/W */
> > > --
> > > 2.20.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ