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]
Message-ID: <CAPDyKFr0xat4T=TpQ7bhbGULsV-8JLfRkXj_4V3q_Odxo+h9tg@mail.gmail.com>
Date:   Tue, 15 Mar 2022 14:41:59 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Marc Mattmüller <marc.mattmueller@...module.com>
Cc:     "avri.altman@....com" <avri.altman@....com>,
        "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, 15 Mar 2022 at 13:40, Marc Mattmüller
<Marc.Mattmueller@...module.com> wrote:
>
> 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.

Okay, that sounds reasonable - and thanks for clarifying.

Kind regards
Uffe

>
> 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