[PATCH v47 00/19] hw/sd/sdcard: Add eMMC support

Philippe Mathieu-Daudé posted 19 patches 4 months, 2 weeks ago
Failed in applying to current master (apply log)
There is a newer version of this series
include/hw/sd/sd.h |   4 +
hw/sd/sd.c         | 424 ++++++++++++++++++++++++++++++++++++++++++++-
hw/sd/trace-events |   3 +
3 files changed, 425 insertions(+), 6 deletions(-)
[PATCH v47 00/19] hw/sd/sdcard: Add eMMC support
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
Since v42:
- Stick to spec v4.3 (re-simplified EXT_CSD register & migrate)
- Fill CID register
- Few changes to CSD register
- Implement 'boot-mode' reset timing
- Add 'boot-size' property

Change required for aspeed branch:
-- >8 --
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 8c0e36badd..563816b710 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -344,3 +344,3 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
         if (emmc) {
-            qdev_prop_set_uint8(card, "boot-config", boot_emmc ? 0x48 : 0x0);
+            qdev_prop_set_uint64(card, "boot-size", 1 * MiB);
         }
(I'm still reluctant to merge patches 16-18)...
---

Cédric Le Goater (2):
  hw/sd/sdcard: Add emmc_cmd_SET_RELATIVE_ADDR handler (CMD3)
  hw/sd/sdcard: Fix SET_BLOCK_COUNT command argument on eMMC (CMD23)

Joel Stanley (3):
  hw/sd/sdcard: Support boot area in emmc image
  hw/sd/sdcard: Subtract bootarea size from blk
  hw/sd/sdcard: Add boot config support

Luc Michel (1):
  hw/sd/sdcard: Implement eMMC sleep state (CMD5)

Philippe Mathieu-Daudé (11):
  hw/sd/sdcard: Basis for eMMC support
  hw/sd/sdcard: Register generic command handlers
  hw/sd/sdcard: Register unimplemented command handlers
  hw/sd/sdcard: Implement emmc_set_cid()
  hw/sd/sdcard: Implement emmc_set_csd()
  hw/sd/sdcard: Add mmc_cmd_PROGRAM_CID handler (CMD26)
  hw/sd/sdcard: Add eMMC 'boot-size' property
  hw/sd/sdcard: Simplify EXT_CSD values for spec v4.3
  hw/sd/sdcard: Migrate ExtCSD 'modes' register
  hw/sd/sdcard: Implement eMMC 'boot-mode'
  hw/sd/sdcard: Enable TYPE_EMMC card model

Sai Pavan Boddu (1):
  hw/sd/sdcard: Add mmc SWITCH function support (CMD6)

Vincent Palatin (1):
  hw/sd/sdcard: Add emmc_cmd_SEND_EXT_CSD handler (CMD8)

 include/hw/sd/sd.h |   4 +
 hw/sd/sd.c         | 424 ++++++++++++++++++++++++++++++++++++++++++++-
 hw/sd/trace-events |   3 +
 3 files changed, 425 insertions(+), 6 deletions(-)

-- 
2.41.0


Re: [PATCH v47 00/19] hw/sd/sdcard: Add eMMC support
Posted by Cédric Le Goater 4 months, 2 weeks ago
On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
> Since v42:
> - Stick to spec v4.3 (re-simplified EXT_CSD register & migrate)
> - Fill CID register
> - Few changes to CSD register
> - Implement 'boot-mode' reset timing
> - Add 'boot-size' property
> 
> Change required for aspeed branch:
> -- >8 --
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 8c0e36badd..563816b710 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -344,3 +344,3 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
>           if (emmc) {
> -            qdev_prop_set_uint8(card, "boot-config", boot_emmc ? 0x48 : 0x0);
> +            qdev_prop_set_uint64(card, "boot-size", 1 * MiB);
>           }
> (I'm still reluctant to merge patches 16-18)...

Then, please drop all changes related to the boot partitions. I will keep
the original patches in my tree and address the feature when I have time.
TYPE_EMMC is already great to have.

Thanks,

C.



> ---
> 
> Cédric Le Goater (2):
>    hw/sd/sdcard: Add emmc_cmd_SET_RELATIVE_ADDR handler (CMD3)
>    hw/sd/sdcard: Fix SET_BLOCK_COUNT command argument on eMMC (CMD23)
> 
> Joel Stanley (3):
>    hw/sd/sdcard: Support boot area in emmc image
>    hw/sd/sdcard: Subtract bootarea size from blk
>    hw/sd/sdcard: Add boot config support
> 
> Luc Michel (1):
>    hw/sd/sdcard: Implement eMMC sleep state (CMD5)
> 
> Philippe Mathieu-Daudé (11):
>    hw/sd/sdcard: Basis for eMMC support
>    hw/sd/sdcard: Register generic command handlers
>    hw/sd/sdcard: Register unimplemented command handlers
>    hw/sd/sdcard: Implement emmc_set_cid()
>    hw/sd/sdcard: Implement emmc_set_csd()
>    hw/sd/sdcard: Add mmc_cmd_PROGRAM_CID handler (CMD26)
>    hw/sd/sdcard: Add eMMC 'boot-size' property
>    hw/sd/sdcard: Simplify EXT_CSD values for spec v4.3
>    hw/sd/sdcard: Migrate ExtCSD 'modes' register
>    hw/sd/sdcard: Implement eMMC 'boot-mode'
>    hw/sd/sdcard: Enable TYPE_EMMC card model
> 
> Sai Pavan Boddu (1):
>    hw/sd/sdcard: Add mmc SWITCH function support (CMD6)
> 
> Vincent Palatin (1):
>    hw/sd/sdcard: Add emmc_cmd_SEND_EXT_CSD handler (CMD8)
> 
>   include/hw/sd/sd.h |   4 +
>   hw/sd/sd.c         | 424 ++++++++++++++++++++++++++++++++++++++++++++-
>   hw/sd/trace-events |   3 +
>   3 files changed, 425 insertions(+), 6 deletions(-)
> 


Re: [PATCH v47 00/19] hw/sd/sdcard: Add eMMC support
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
On 9/7/24 17:58, Cédric Le Goater wrote:
> On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
>> Since v42:
>> - Stick to spec v4.3 (re-simplified EXT_CSD register & migrate)
>> - Fill CID register
>> - Few changes to CSD register
>> - Implement 'boot-mode' reset timing
>> - Add 'boot-size' property
>>
>> Change required for aspeed branch:
>> -- >8 --
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 8c0e36badd..563816b710 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -344,3 +344,3 @@ static void sdhci_attach_drive(SDHCIState *sdhci, 
>> DriveInfo *dinfo, bool emmc,
>>           if (emmc) {
>> -            qdev_prop_set_uint8(card, "boot-config", boot_emmc ? 0x48 
>> : 0x0);
>> +            qdev_prop_set_uint64(card, "boot-size", 1 * MiB);
>>           }
>> (I'm still reluctant to merge patches 16-18)...
> 
> Then, please drop all changes related to the boot partitions. I will keep
> the original patches in my tree and address the feature when I have time.
> TYPE_EMMC is already great to have.

As you mentioned on today's community call, eMMC is a chipset soldered
on a board, so our boards exactly knows what size to expect on the card,
and whether boot partitions are present or not. I find the way of
building the drive image described in patch #16 cumbersome, but I'm OK
to make some concession on it, since "it works" as it. If necessary
we'll improve and deprecate the current properties. I'll repost and
plan to merge as-is (modulo review comments).

Regards,

Phil.

Re: [PATCH v47 00/19] hw/sd/sdcard: Add eMMC support
Posted by Cédric Le Goater 4 months, 2 weeks ago
On 7/9/24 11:39 PM, Philippe Mathieu-Daudé wrote:
> On 9/7/24 17:58, Cédric Le Goater wrote:
>> On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
>>> Since v42:
>>> - Stick to spec v4.3 (re-simplified EXT_CSD register & migrate)
>>> - Fill CID register
>>> - Few changes to CSD register
>>> - Implement 'boot-mode' reset timing
>>> - Add 'boot-size' property
>>>
>>> Change required for aspeed branch:
>>> -- >8 --
>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>> index 8c0e36badd..563816b710 100644
>>> --- a/hw/arm/aspeed.c
>>> +++ b/hw/arm/aspeed.c
>>> @@ -344,3 +344,3 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
>>>           if (emmc) {
>>> -            qdev_prop_set_uint8(card, "boot-config", boot_emmc ? 0x48 : 0x0);
>>> +            qdev_prop_set_uint64(card, "boot-size", 1 * MiB);
>>>           }
>>> (I'm still reluctant to merge patches 16-18)...
>>
>> Then, please drop all changes related to the boot partitions. I will keep
>> the original patches in my tree and address the feature when I have time.
>> TYPE_EMMC is already great to have.
> 
> As you mentioned on today's community call, eMMC is a chipset soldered
> on a board, so our boards exactly knows what size to expect on the card,
> and whether boot partitions are present or not. 

My remark was on the use of 3 blockdevs to represent the eMMC contents.
1 should be enough.

Having a "boot-size" property to set the BOOT_SIZE_MULT register is fine.
The Aspeed model does some assumption today when installing the first
boot area as a boot ROM :

     aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);

The "boot-size" property will clarify the settings. Please keep it.

Other topic :

      ROMs need to be installed early and IIRC, user creatable devices
      are not available at that time. So, when the flash devices are be
      defined on the command line with :

         -blockdev node-name=fmc0,driver=file,filename=./flash.img \
         -device mx66u51235f,cs=0x0,bus=ssi.0,drive=fmc0 \

      the ROM can not be installed and the initial boot will execute
      in place, using SPI transactions to fetch instructions which is
      slower. To install a ROM, the -drive API is required.

> I find the way of building the drive image described in patch #16 cumbersome, 

I agree but that's the layout on real HW. The flash layouts are even more
complex.

> but I'm OK
> to make some concession on it, since "it works" as it. If necessary
> we'll improve and deprecate the current properties. 

I don't think we will need to. The properties make sense.

> I'll repost and plan to merge as-is (modulo review comments).

OK. I will rebase then.

Thanks,

C.


Re: [PATCH v47 00/19] hw/sd/sdcard: Add eMMC support
Posted by Cédric Le Goater 4 months, 2 weeks ago
On 7/10/24 6:58 AM, Cédric Le Goater wrote:
> On 7/9/24 11:39 PM, Philippe Mathieu-Daudé wrote:
>> On 9/7/24 17:58, Cédric Le Goater wrote:
>>> On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
>>>> Since v42:
>>>> - Stick to spec v4.3 (re-simplified EXT_CSD register & migrate)
>>>> - Fill CID register
>>>> - Few changes to CSD register
>>>> - Implement 'boot-mode' reset timing
>>>> - Add 'boot-size' property
>>>>
>>>> Change required for aspeed branch:
>>>> -- >8 --
>>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>>> index 8c0e36badd..563816b710 100644
>>>> --- a/hw/arm/aspeed.c
>>>> +++ b/hw/arm/aspeed.c
>>>> @@ -344,3 +344,3 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo, bool emmc,
>>>>           if (emmc) {
>>>> -            qdev_prop_set_uint8(card, "boot-config", boot_emmc ? 0x48 : 0x0);
>>>> +            qdev_prop_set_uint64(card, "boot-size", 1 * MiB);
>>>>           }
>>>> (I'm still reluctant to merge patches 16-18)...
>>>
>>> Then, please drop all changes related to the boot partitions. I will keep
>>> the original patches in my tree and address the feature when I have time.
>>> TYPE_EMMC is already great to have.
>>
>> As you mentioned on today's community call, eMMC is a chipset soldered
>> on a board, so our boards exactly knows what size to expect on the card,
>> and whether boot partitions are present or not. 
> 
> My remark was on the use of 3 blockdevs to represent the eMMC contents.
> 1 should be enough.
> 
> Having a "boot-size" property to set the BOOT_SIZE_MULT register is fine.
> The Aspeed model does some assumption today when installing the first
> boot area as a boot ROM :
> 
>      aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(emmc0), 64 * KiB);
> 
> The "boot-size" property will clarify the settings. Please keep it.

64 * KiB is not related to the eMMC partition size. It is a SRAM
limit in which the SoC loads the boot data.

Thanks,

C.
  

[PATCH v47 01/19] hw/sd/sdcard: Basis for eMMC support
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Add basis for supporting eMMC.

Since eMMC are soldered on boards, it is not user-creatable.

Currently TYPE_EMMC is just a stub, so disabled (marked abstract).

RCA register is initialized to 1, per spec v4.3,
chapter 8.5 "RCA register":

  The default value of the RCA register is 0x0001.
  The value 0x0000 is reserved to set all cards into
  the Stand-by State with CMD7.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/sd/sd.h |  3 +++
 hw/sd/sd.c         | 50 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 0d6d9e452b..d35a839f5e 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -96,6 +96,9 @@ OBJECT_DECLARE_TYPE(SDState, SDCardClass, SD_CARD)
 #define TYPE_SD_CARD_SPI "sd-card-spi"
 DECLARE_INSTANCE_CHECKER(SDState, SD_CARD_SPI, TYPE_SD_CARD_SPI)
 
+#define TYPE_EMMC "emmc"
+DECLARE_INSTANCE_CHECKER(SDState, EMMC, TYPE_EMMC)
+
 struct SDCardClass {
     /*< private >*/
     DeviceClass parent_class;
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d6a07f0ade..91a73aea8d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2,6 +2,8 @@
  * SD Memory Card emulation as defined in the "SD Memory Card Physical
  * layer specification, Version 2.00."
  *
+ * eMMC emulation defined in "JEDEC Standard No. 84-A43"
+ *
  * Copyright (c) 2006 Andrzej Zaborowski  <balrog@zabor.org>
  * Copyright (c) 2007 CodeSourcery
  * Copyright (c) 2018 Philippe Mathieu-Daudé <f4bug@amsat.org>
@@ -169,12 +171,18 @@ struct SDState {
 static void sd_realize(DeviceState *dev, Error **errp);
 
 static const SDProto sd_proto_spi;
+static const SDProto sd_proto_emmc;
 
 static bool sd_is_spi(SDState *sd)
 {
     return sd->proto == &sd_proto_spi;
 }
 
+static bool sd_is_emmc(SDState *sd)
+{
+    return sd->proto == &sd_proto_emmc;
+}
+
 static const char *sd_version_str(enum SDPhySpecificationVersion version)
 {
     static const char *sdphy_version[] = {
@@ -697,7 +705,7 @@ static void sd_reset(DeviceState *dev)
     sd->state = sd_idle_state;
 
     /* card registers */
-    sd->rca = 0x0000;
+    sd->rca = sd_is_emmc(sd) ? 0x0001 : 0x0000;
     sd->size = size;
     sd_set_ocr(sd);
     sd_set_scr(sd);
@@ -2375,6 +2383,13 @@ static const SDProto sd_proto_sd = {
     },
 };
 
+static const SDProto sd_proto_emmc = {
+    /* Only v4.3 is supported */
+    .name = "eMMC",
+    .cmd = {
+    },
+};
+
 static void sd_instance_init(Object *obj)
 {
     SDState *sd = SDMMC_COMMON(obj);
@@ -2446,6 +2461,15 @@ static void sd_realize(DeviceState *dev, Error **errp)
     }
 }
 
+static void emmc_realize(DeviceState *dev, Error **errp)
+{
+    SDState *sd = SDMMC_COMMON(dev);
+
+    sd->spec_version = SD_PHY_SPECv3_01_VERS; /* Actually v4.3 */
+
+    sd_realize(dev, errp);
+}
+
 static Property sdmmc_common_properties[] = {
     DEFINE_PROP_DRIVE("drive", SDState, blk),
     DEFINE_PROP_END_OF_LIST()
@@ -2457,6 +2481,10 @@ static Property sd_properties[] = {
     DEFINE_PROP_END_OF_LIST()
 };
 
+static Property emmc_properties[] = {
+    DEFINE_PROP_END_OF_LIST()
+};
+
 static void sdmmc_common_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -2509,6 +2537,20 @@ static void sd_spi_class_init(ObjectClass *klass, void *data)
     sc->proto = &sd_proto_spi;
 }
 
+static void emmc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
+
+    dc->desc = "eMMC";
+    dc->realize = emmc_realize;
+    device_class_set_props(dc, emmc_properties);
+    /* Reason: Soldered on board */
+    dc->user_creatable = false;
+
+    sc->proto = &sd_proto_emmc;
+}
+
 static const TypeInfo sd_types[] = {
     {
         .name           = TYPE_SDMMC_COMMON,
@@ -2530,6 +2572,12 @@ static const TypeInfo sd_types[] = {
         .parent         = TYPE_SD_CARD,
         .class_init     = sd_spi_class_init,
     },
+    {
+        .name           = TYPE_EMMC,
+        .parent         = TYPE_SDMMC_COMMON,
+        .class_init     = emmc_class_init,
+        .abstract       = true, /* FIXME: Remove once model fully functional */
+    },
 };
 
 DEFINE_TYPES(sd_types)
-- 
2.41.0


[PATCH v47 02/19] hw/sd/sdcard: Register generic command handlers
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20240628070216.92609-84-philmd@linaro.org>
---
 hw/sd/sd.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 91a73aea8d..eb50862adb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2387,6 +2387,29 @@ static const SDProto sd_proto_emmc = {
     /* Only v4.3 is supported */
     .name = "eMMC",
     .cmd = {
+        [0]  = {0,  sd_bc,   "GO_IDLE_STATE", sd_cmd_GO_IDLE_STATE},
+        [1]  = {0,  sd_bcr,  "SEND_OP_COND", sd_cmd_SEND_OP_COND},
+        [2]  = {0,  sd_bcr,  "ALL_SEND_CID", sd_cmd_ALL_SEND_CID},
+        [7]  = {0,  sd_ac,   "(DE)SELECT_CARD", sd_cmd_DE_SELECT_CARD},
+        [9]  = {0,  sd_ac,   "SEND_CSD", sd_cmd_SEND_CSD},
+        [10] = {0,  sd_ac,   "SEND_CID", sd_cmd_SEND_CID},
+        [12] = {0,  sd_ac,   "STOP_TRANSMISSION", sd_cmd_STOP_TRANSMISSION},
+        [13] = {0,  sd_ac,   "SEND_STATUS", sd_cmd_SEND_STATUS},
+        [15] = {0,  sd_ac,   "GO_INACTIVE_STATE", sd_cmd_GO_INACTIVE_STATE},
+        [16] = {2,  sd_ac,   "SET_BLOCKLEN", sd_cmd_SET_BLOCKLEN},
+        [17] = {2,  sd_adtc, "READ_SINGLE_BLOCK", sd_cmd_READ_SINGLE_BLOCK},
+        [23] = {2,  sd_ac,   "SET_BLOCK_COUNT", sd_cmd_SET_BLOCK_COUNT},
+        [24] = {4,  sd_adtc, "WRITE_SINGLE_BLOCK", sd_cmd_WRITE_SINGLE_BLOCK},
+        [27] = {4,  sd_adtc, "PROGRAM_CSD", sd_cmd_PROGRAM_CSD},
+        [28] = {6,  sd_ac,   "SET_WRITE_PROT", sd_cmd_SET_WRITE_PROT},
+        [29] = {6,  sd_ac,   "CLR_WRITE_PROT", sd_cmd_CLR_WRITE_PROT},
+        [30] = {6,  sd_adtc, "SEND_WRITE_PROT", sd_cmd_SEND_WRITE_PROT},
+        [35] = {5,  sd_ac,   "ERASE_WR_BLK_START", sd_cmd_ERASE_WR_BLK_START},
+        [36] = {5,  sd_ac,   "ERASE_WR_BLK_END", sd_cmd_ERASE_WR_BLK_END},
+        [38] = {5,  sd_ac,   "ERASE", sd_cmd_ERASE},
+        [42] = {7,  sd_adtc, "LOCK_UNLOCK", sd_cmd_LOCK_UNLOCK},
+        [55] = {8,  sd_ac,   "APP_CMD", sd_cmd_APP_CMD},
+        [56] = {8,  sd_adtc, "GEN_CMD", sd_cmd_GEN_CMD},
     },
 };
 
-- 
2.41.0


[PATCH v47 03/19] hw/sd/sdcard: Register unimplemented command handlers
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
Per the spec v4.3 these commands are mandatory,
but we don't implement them.

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20240628070216.92609-85-philmd@linaro.org>
---
 hw/sd/sd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index eb50862adb..097c9cc61f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2390,24 +2390,33 @@ static const SDProto sd_proto_emmc = {
         [0]  = {0,  sd_bc,   "GO_IDLE_STATE", sd_cmd_GO_IDLE_STATE},
         [1]  = {0,  sd_bcr,  "SEND_OP_COND", sd_cmd_SEND_OP_COND},
         [2]  = {0,  sd_bcr,  "ALL_SEND_CID", sd_cmd_ALL_SEND_CID},
+        [4]  = {0,  sd_bc,   "SEND_DSR", sd_cmd_unimplemented},
         [7]  = {0,  sd_ac,   "(DE)SELECT_CARD", sd_cmd_DE_SELECT_CARD},
         [9]  = {0,  sd_ac,   "SEND_CSD", sd_cmd_SEND_CSD},
         [10] = {0,  sd_ac,   "SEND_CID", sd_cmd_SEND_CID},
+        [11] = {1,  sd_adtc, "READ_DAT_UNTIL_STOP", sd_cmd_unimplemented},
         [12] = {0,  sd_ac,   "STOP_TRANSMISSION", sd_cmd_STOP_TRANSMISSION},
         [13] = {0,  sd_ac,   "SEND_STATUS", sd_cmd_SEND_STATUS},
+        [14] = {0,  sd_adtc, "BUSTEST_R", sd_cmd_unimplemented},
         [15] = {0,  sd_ac,   "GO_INACTIVE_STATE", sd_cmd_GO_INACTIVE_STATE},
         [16] = {2,  sd_ac,   "SET_BLOCKLEN", sd_cmd_SET_BLOCKLEN},
         [17] = {2,  sd_adtc, "READ_SINGLE_BLOCK", sd_cmd_READ_SINGLE_BLOCK},
+        [19] = {0,  sd_adtc, "BUSTEST_W", sd_cmd_unimplemented},
+        [20] = {3,  sd_adtc, "WRITE_DAT_UNTIL_STOP", sd_cmd_unimplemented},
         [23] = {2,  sd_ac,   "SET_BLOCK_COUNT", sd_cmd_SET_BLOCK_COUNT},
         [24] = {4,  sd_adtc, "WRITE_SINGLE_BLOCK", sd_cmd_WRITE_SINGLE_BLOCK},
         [27] = {4,  sd_adtc, "PROGRAM_CSD", sd_cmd_PROGRAM_CSD},
         [28] = {6,  sd_ac,   "SET_WRITE_PROT", sd_cmd_SET_WRITE_PROT},
         [29] = {6,  sd_ac,   "CLR_WRITE_PROT", sd_cmd_CLR_WRITE_PROT},
         [30] = {6,  sd_adtc, "SEND_WRITE_PROT", sd_cmd_SEND_WRITE_PROT},
+        [31] = {6,  sd_adtc, "SEND_WRITE_PROT_TYPE", sd_cmd_unimplemented},
         [35] = {5,  sd_ac,   "ERASE_WR_BLK_START", sd_cmd_ERASE_WR_BLK_START},
         [36] = {5,  sd_ac,   "ERASE_WR_BLK_END", sd_cmd_ERASE_WR_BLK_END},
         [38] = {5,  sd_ac,   "ERASE", sd_cmd_ERASE},
+        [39] = {9,  sd_ac,   "FAST_IO", sd_cmd_unimplemented},
+        [40] = {9,  sd_bcr,  "GO_IRQ_STATE", sd_cmd_unimplemented},
         [42] = {7,  sd_adtc, "LOCK_UNLOCK", sd_cmd_LOCK_UNLOCK},
+        [49] = {0,  sd_adtc, "SET_TIME", sd_cmd_unimplemented},
         [55] = {8,  sd_ac,   "APP_CMD", sd_cmd_APP_CMD},
         [56] = {8,  sd_adtc, "GEN_CMD", sd_cmd_GEN_CMD},
     },
-- 
2.41.0


[PATCH v47 04/19] hw/sd/sdcard: Implement emmc_set_cid()
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
eMMC CID register is slightly different from SD:
- One extra PNM (5 -> 6)
- MDT is only 1 byte (2 -> 1).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 097c9cc61f..2d737a836f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -446,6 +446,23 @@ static void sd_set_cid(SDState *sd)
     sd->cid[15] = (sd_crc7(sd->cid, 15) << 1) | 1;
 }
 
+static void emmc_set_cid(SDState *sd)
+{
+    sd->cid[0] = MID;       /* Fake card manufacturer ID (MID) */
+    sd->cid[1] = 0b01;      /* CBX: BGA */
+    sd->cid[2] = OID[0];    /* OEM/Application ID (OID) */
+    sd->cid[3] = PNM[0];    /* Fake product name (PNM) */
+    sd->cid[4] = PNM[1];
+    sd->cid[5] = PNM[2];
+    sd->cid[6] = PNM[3];
+    sd->cid[7] = PNM[4];
+    sd->cid[8] = PNM[4];
+    sd->cid[9] = PRV;       /* Fake product revision (PRV) */
+    stl_be_p(&sd->cid[10], 0xdeadbeef); /* Fake serial number (PSN) */
+    sd->cid[14] = (MDT_MON << 4) | (MDT_YR - 1997); /* Manufacture date (MDT) */
+    sd->cid[15] = (sd_crc7(sd->cid, 15) << 1) | 1;
+}
+
 /* Card-Specific Data register */
 
 #define HWBLOCK_SHIFT   9        /* 512 bytes */
@@ -2581,6 +2598,8 @@ static void emmc_class_init(ObjectClass *klass, void *data)
     dc->user_creatable = false;
 
     sc->proto = &sd_proto_emmc;
+
+    sc->set_cid = emmc_set_cid;
 }
 
 static const TypeInfo sd_types[] = {
-- 
2.41.0


[PATCH v47 05/19] hw/sd/sdcard: Implement emmc_set_csd()
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
eMMC CSD register is very similar to SD one.
Most notable change: the version announced is v4.3.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
TODO: comment magic values?
---
 hw/sd/sd.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2d737a836f..f580c6b2ae 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -476,6 +476,44 @@ static const uint8_t sd_csd_rw_mask[16] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xfc, 0xfe,
 };
 
+static void emmc_set_csd(SDState *sd, uint64_t size)
+{
+    int hwblock_shift = HWBLOCK_SHIFT;
+    uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
+    uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1;
+
+    sd->csd[0] = (2 << 6) | (4 << 2);
+    sd->csd[1] = 0x07;
+    sd->csd[2] = 0x00;
+    sd->csd[3] = 0x32;
+    sd->csd[4] = 0x0f;
+    if (size <= 2 * GiB) {
+        /* use 1k blocks */
+        uint32_t csize1k = (size >> (CMULT_SHIFT + 10)) - 1;
+        sd->csd[5] = 0x5a;
+        sd->csd[6] = 0x80 | ((csize1k >> 10) & 0xf);
+        sd->csd[7] = (csize1k >> 2) & 0xff;
+    } else { /* >= 2GB : size stored in ext CSD, block addressing */
+        sd->csd[5] = 0x59;
+        sd->csd[6] = 0x8f;
+        sd->csd[7] = 0xff;
+        sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
+    }
+    sd->csd[8] = 0xff;
+    sd->csd[9] = 0xfc |     /* Max. write current */
+        ((CMULT_SHIFT - 2) >> 1);
+    sd->csd[10] = 0x40 |    /* Erase sector size */
+        (((CMULT_SHIFT - 2) << 7) & 0x80) | (sectsize >> 1);
+    sd->csd[11] = 0x00 |    /* Write protect group size */
+        ((sectsize << 7) & 0x80) | wpsize;
+    sd->csd[12] = 0x90 |    /* Write speed factor */
+        (hwblock_shift >> 2);
+    sd->csd[13] = 0x20 |    /* Max. write data block length */
+        ((hwblock_shift << 6) & 0xc0);
+    sd->csd[14] = 0x00;
+    sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
+}
+
 static void sd_set_csd(SDState *sd, uint64_t size)
 {
     int hwblock_shift = HWBLOCK_SHIFT;
@@ -2600,6 +2638,7 @@ static void emmc_class_init(ObjectClass *klass, void *data)
     sc->proto = &sd_proto_emmc;
 
     sc->set_cid = emmc_set_cid;
+    sc->set_csd = emmc_set_csd;
 }
 
 static const TypeInfo sd_types[] = {
-- 
2.41.0


[PATCH v47 06/19] hw/sd/sdcard: Add emmc_cmd_SET_RELATIVE_ADDR handler (CMD3)
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
From: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20240628070216.92609-86-philmd@linaro.org>
---
 hw/sd/sd.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f580c6b2ae..83d45c897f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1280,6 +1280,20 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
     }
 }
 
+static sd_rsp_type_t emmc_cmd_SET_RELATIVE_ADDR(SDState *sd, SDRequest req)
+{
+    switch (sd->state) {
+    case sd_identification_state:
+    case sd_standby_state:
+        sd->state = sd_standby_state;
+        sd_set_rca(sd, req.arg >> 16);
+        return sd_r1;
+
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 /* CMD6 */
 static sd_rsp_type_t sd_cmd_SWITCH_FUNCTION(SDState *sd, SDRequest req)
 {
@@ -2445,6 +2459,7 @@ static const SDProto sd_proto_emmc = {
         [0]  = {0,  sd_bc,   "GO_IDLE_STATE", sd_cmd_GO_IDLE_STATE},
         [1]  = {0,  sd_bcr,  "SEND_OP_COND", sd_cmd_SEND_OP_COND},
         [2]  = {0,  sd_bcr,  "ALL_SEND_CID", sd_cmd_ALL_SEND_CID},
+        [3]  = {0,  sd_ac,   "SET_RELATIVE_ADDR", emmc_cmd_SET_RELATIVE_ADDR},
         [4]  = {0,  sd_bc,   "SEND_DSR", sd_cmd_unimplemented},
         [7]  = {0,  sd_ac,   "(DE)SELECT_CARD", sd_cmd_DE_SELECT_CARD},
         [9]  = {0,  sd_ac,   "SEND_CSD", sd_cmd_SEND_CSD},
-- 
2.41.0


[PATCH v47 07/19] hw/sd/sdcard: Fix SET_BLOCK_COUNT command argument on eMMC (CMD23)
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
From: Cédric Le Goater <clg@kaod.org>

The number of blocks is defined in the lower bits [15:0].

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20240628070216.92609-88-philmd@linaro.org>
---
 hw/sd/sd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 83d45c897f..216d4dfa89 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1533,6 +1533,9 @@ static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd, SDRequest req)
     }
 
     sd->multi_blk_cnt = req.arg;
+    if (sd_is_emmc(sd)) {
+        sd->multi_blk_cnt &= 0xffff;
+    }
     trace_sdcard_set_block_count(sd->multi_blk_cnt);
 
     return sd_r1;
-- 
2.41.0


[PATCH v47 08/19] hw/sd/sdcard: Add mmc_cmd_PROGRAM_CID handler (CMD26)
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240628070216.92609-89-philmd@linaro.org>
---
 hw/sd/sd.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 216d4dfa89..05c1b85476 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1568,6 +1568,12 @@ static sd_rsp_type_t sd_cmd_WRITE_SINGLE_BLOCK(SDState *sd, SDRequest req)
     return sd_cmd_to_receivingdata(sd, req, addr, sd->blk_len);
 }
 
+/* CMD26 */
+static sd_rsp_type_t mmc_cmd_PROGRAM_CID(SDState *sd, SDRequest req)
+{
+    return sd_cmd_to_receivingdata(sd, req, 0, sizeof(sd->cid));
+}
+
 /* CMD27 */
 static sd_rsp_type_t sd_cmd_PROGRAM_CSD(SDState *sd, SDRequest req)
 {
@@ -1917,9 +1923,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 26:  /* CMD26:  PROGRAM_CID */
-        return sd_cmd_to_receivingdata(sd, req, 0, sizeof(sd->cid));
-
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
         return sd_illegal;
@@ -2478,6 +2481,7 @@ static const SDProto sd_proto_emmc = {
         [20] = {3,  sd_adtc, "WRITE_DAT_UNTIL_STOP", sd_cmd_unimplemented},
         [23] = {2,  sd_ac,   "SET_BLOCK_COUNT", sd_cmd_SET_BLOCK_COUNT},
         [24] = {4,  sd_adtc, "WRITE_SINGLE_BLOCK", sd_cmd_WRITE_SINGLE_BLOCK},
+        [26] = {4,  sd_adtc, "PROGRAM_CID", mmc_cmd_PROGRAM_CID},
         [27] = {4,  sd_adtc, "PROGRAM_CSD", sd_cmd_PROGRAM_CSD},
         [28] = {6,  sd_ac,   "SET_WRITE_PROT", sd_cmd_SET_WRITE_PROT},
         [29] = {6,  sd_ac,   "CLR_WRITE_PROT", sd_cmd_CLR_WRITE_PROT},
-- 
2.41.0


[PATCH v47 09/19] hw/sd/sdcard: Implement eMMC sleep state (CMD5)
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
From: Luc Michel <luc.michel@amd.com>

The JEDEC standards specifies a sleep state where the eMMC won't answer
any command appart from RESET and WAKEUP and go to low power state.
Implement this state and the corresponding command number 5.

Signed-off-by: Luc Michel <luc.michel@amd.com>
Signed-off-by: Francisco Iglesias <francisco.iglesias@amd.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240628070216.92609-90-philmd@linaro.org>
---
 hw/sd/sd.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 05c1b85476..adba53e822 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1236,8 +1236,19 @@ static sd_rsp_type_t sd_cmd_to_sendingdata(SDState *sd, SDRequest req,
 /* CMD0 */
 static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
 {
-    sd->state = sd_idle_state;
-    sd_reset(DEVICE(sd));
+    if (sd->state == sd_sleep_state) {
+        switch (req.arg) {
+        case 0x00000000:
+        case 0xf0f0f0f0:
+            break;
+        default:
+            return sd_r0;
+        }
+    }
+    if (sd->state != sd_inactive_state) {
+        sd->state = sd_idle_state;
+        sd_reset(DEVICE(sd));
+    }
 
     return sd_is_spi(sd) ? sd_r1 : sd_r0;
 }
@@ -1294,6 +1305,30 @@ static sd_rsp_type_t emmc_cmd_SET_RELATIVE_ADDR(SDState *sd, SDRequest req)
     }
 }
 
+/* CMD5 */
+static sd_rsp_type_t emmc_cmd_sleep_awake(SDState *sd, SDRequest req)
+{
+    bool do_sleep = extract32(req.arg, 15, 1);
+
+    switch (sd->state) {
+    case sd_sleep_state:
+        if (!do_sleep) {
+            /* Awake */
+            sd->state = sd_standby_state;
+        }
+        return sd_r1b;
+
+    case sd_standby_state:
+        if (do_sleep) {
+            sd->state = sd_sleep_state;
+        }
+        return sd_r1b;
+
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 /* CMD6 */
 static sd_rsp_type_t sd_cmd_SWITCH_FUNCTION(SDState *sd, SDRequest req)
 {
@@ -1696,6 +1731,7 @@ static sd_rsp_type_t sd_cmd_APP_CMD(SDState *sd, SDRequest req)
     case sd_ready_state:
     case sd_identification_state:
     case sd_inactive_state:
+    case sd_sleep_state:
         return sd_invalid_state_for_cmd(sd, req);
     case sd_idle_state:
         if (!sd_is_spi(sd) && sd_req_get_rca(sd, req) != 0x0000) {
@@ -2018,6 +2054,12 @@ int sd_do_command(SDState *sd, SDRequest *req,
         req->cmd &= 0x3f;
     }
 
+    if (sd->state == sd_sleep_state && req->cmd) {
+        qemu_log_mask(LOG_GUEST_ERROR, "SD: Card is sleeping\n");
+        rtype = sd_r0;
+        goto send_response;
+    }
+
     if (sd->card_status & CARD_IS_LOCKED) {
         if (!cmd_valid_while_locked(sd, req->cmd)) {
             sd->card_status |= ILLEGAL_COMMAND;
@@ -2467,6 +2509,7 @@ static const SDProto sd_proto_emmc = {
         [2]  = {0,  sd_bcr,  "ALL_SEND_CID", sd_cmd_ALL_SEND_CID},
         [3]  = {0,  sd_ac,   "SET_RELATIVE_ADDR", emmc_cmd_SET_RELATIVE_ADDR},
         [4]  = {0,  sd_bc,   "SEND_DSR", sd_cmd_unimplemented},
+        [5]  = {0,  sd_ac,   "SLEEP/AWAKE", emmc_cmd_sleep_awake},
         [7]  = {0,  sd_ac,   "(DE)SELECT_CARD", sd_cmd_DE_SELECT_CARD},
         [9]  = {0,  sd_ac,   "SEND_CSD", sd_cmd_SEND_CSD},
         [10] = {0,  sd_ac,   "SEND_CID", sd_cmd_SEND_CID},
-- 
2.41.0


[PATCH v47 10/19] hw/sd/sdcard: Add emmc_cmd_SEND_EXT_CSD handler (CMD8)
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
From: Vincent Palatin <vpalatin@chromium.org>

The parameters mimick a real 4GB eMMC, but it can be set to various
sizes. Initially from Vincent Palatin <vpalatin@chromium.org>

eMMC CSD is similar to SD with an option to refer EXT_CSD for larger
devices.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
[PMD: Remove deprecated EXT_CSD_SEC_ERASE_MULT/EXT_CSD_SEC_TRIM_MULT]
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index adba53e822..c809961418 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -124,6 +124,7 @@ struct SDState {
     uint16_t rca;
     uint32_t card_status;
     uint8_t sd_status[64];
+    uint8_t ext_csd[512];
 
     /* Static properties */
 
@@ -476,6 +477,50 @@ static const uint8_t sd_csd_rw_mask[16] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xfc, 0xfe,
 };
 
+static void mmc_set_ext_csd(SDState *sd, uint64_t size)
+{
+    uint32_t sectcount = size >> HWBLOCK_SHIFT;
+
+    memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
+
+    sd->ext_csd[EXT_CSD_S_CMD_SET] = 0b1; /* supported command sets */
+    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
+    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations */
+    sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
+    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
+    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */
+    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
+    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
+    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
+    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
+    sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x01; /* HC erase timeout */
+    sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
+    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
+    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
+    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
+    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
+    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
+    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
+    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
+    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
+    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
+    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
+    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
+    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
+    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
+    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
+    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1;
+    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1;
+    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0x7;
+    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2;
+    sd->ext_csd[EXT_CSD_REV] = 0x5;
+    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
+    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3;
+    sd->ext_csd[159] = 0x00; /* Max enhanced area size */
+    sd->ext_csd[158] = 0x00; /* ... */
+    sd->ext_csd[157] = 0xEC; /* ... */
+}
+
 static void emmc_set_csd(SDState *sd, uint64_t size)
 {
     int hwblock_shift = HWBLOCK_SHIFT;
@@ -512,6 +557,7 @@ static void emmc_set_csd(SDState *sd, uint64_t size)
         ((hwblock_shift << 6) & 0xc0);
     sd->csd[14] = 0x00;
     sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
+    mmc_set_ext_csd(sd, size);
 }
 
 static void sd_set_csd(SDState *sd, uint64_t size)
@@ -1405,6 +1451,17 @@ static sd_rsp_type_t sd_cmd_SEND_IF_COND(SDState *sd, SDRequest req)
     return sd_r7;
 }
 
+/* CMD8 */
+static sd_rsp_type_t emmc_cmd_SEND_EXT_CSD(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_transfer_state) {
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+
+    return sd_cmd_to_sendingdata(sd, req, sd_req_get_address(sd, req),
+                                 sd->ext_csd, sizeof(sd->ext_csd));
+}
+
 /* CMD9 */
 static sd_rsp_type_t spi_cmd_SEND_CSD(SDState *sd, SDRequest req)
 {
@@ -2334,6 +2391,7 @@ uint8_t sd_read_byte(SDState *sd)
                            sd->data_offset, sd->data_size, io_len);
     switch (sd->current_cmd) {
     case 6:  /* CMD6:   SWITCH_FUNCTION */
+    case 8:  /* CMD8:   SEND_EXT_CSD */
     case 9:  /* CMD9:   SEND_CSD */
     case 10: /* CMD10:  SEND_CID */
     case 13: /* ACMD13: SD_STATUS */
@@ -2511,6 +2569,7 @@ static const SDProto sd_proto_emmc = {
         [4]  = {0,  sd_bc,   "SEND_DSR", sd_cmd_unimplemented},
         [5]  = {0,  sd_ac,   "SLEEP/AWAKE", emmc_cmd_sleep_awake},
         [7]  = {0,  sd_ac,   "(DE)SELECT_CARD", sd_cmd_DE_SELECT_CARD},
+        [8]  = {0,  sd_adtc, "SEND_EXT_CSD", emmc_cmd_SEND_EXT_CSD},
         [9]  = {0,  sd_ac,   "SEND_CSD", sd_cmd_SEND_CSD},
         [10] = {0,  sd_ac,   "SEND_CID", sd_cmd_SEND_CID},
         [11] = {1,  sd_adtc, "READ_DAT_UNTIL_STOP", sd_cmd_unimplemented},
-- 
2.41.0


[PATCH v47 11/19] hw/sd/sdcard: Add eMMC 'boot-size' property
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
Avoid hardcoding 1MiB boot size in EXT_CSD_BOOT_MULT,
expose it as QOM property.

By default, do not use any size. Board is responsible
to set the boot size property.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index c809961418..df0e2345c0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -129,6 +129,7 @@ struct SDState {
     /* Static properties */
 
     uint8_t spec_version;
+    uint64_t boot_part_size;
     BlockBackend *blk;
 
     const SDProto *proto;
@@ -490,7 +491,8 @@ static void mmc_set_ext_csd(SDState *sd, uint64_t size)
     sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
     sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */
     sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
-    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
+                                     /* Boot partition size. 128KB unit */
+    sd->ext_csd[EXT_CSD_BOOT_MULT] = sd->boot_part_size / (128 * KiB);
     sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
     sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
     sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x01; /* HC erase timeout */
@@ -2693,6 +2695,7 @@ static Property sd_properties[] = {
 };
 
 static Property emmc_properties[] = {
+    DEFINE_PROP_UINT64("boot-size", SDState, boot_part_size, 0),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.41.0


Re: [PATCH v47 11/19] hw/sd/sdcard: Add eMMC 'boot-size' property
Posted by Cédric Le Goater 4 months, 2 weeks ago
On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
> Avoid hardcoding 1MiB boot size in EXT_CSD_BOOT_MULT,
> expose it as QOM property.
> 
> By default, do not use any size. Board is responsible
> to set the boot size property.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

I would move this patch at the end with the other patches adding
boot support.


Thanks,

C.



> ---
>   hw/sd/sd.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index c809961418..df0e2345c0 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -129,6 +129,7 @@ struct SDState {
>       /* Static properties */
>   
>       uint8_t spec_version;
> +    uint64_t boot_part_size;
>       BlockBackend *blk;
>   
>       const SDProto *proto;
> @@ -490,7 +491,8 @@ static void mmc_set_ext_csd(SDState *sd, uint64_t size)
>       sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
>       sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */
>       sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
> -    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
> +                                     /* Boot partition size. 128KB unit */
> +    sd->ext_csd[EXT_CSD_BOOT_MULT] = sd->boot_part_size / (128 * KiB);
>       sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
>       sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
>       sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x01; /* HC erase timeout */
> @@ -2693,6 +2695,7 @@ static Property sd_properties[] = {
>   };
>   
>   static Property emmc_properties[] = {
> +    DEFINE_PROP_UINT64("boot-size", SDState, boot_part_size, 0),
>       DEFINE_PROP_END_OF_LIST()
>   };
>   


[PATCH v47 12/19] hw/sd/sdcard: Simplify EXT_CSD values for spec v4.3
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
- Set some keys to not defined / implemented:
  . EXT_CSD_HPI_FEATURES
  . EXT_CSD_BKOPS_SUPPORT
  . EXT_CSD_SEC_FEATURE_SUPPORT
  . EXT_CSD_ERASE_TIMEOUT_MULT
  . EXT_CSD_PART_SWITCH_TIME
  . EXT_CSD_OUT_OF_INTERRUPT_TIME

- Simplify:
  . EXT_CSD_ACC_SIZE (6 -> 1)
      16KB of super_page_size -> 512B (BDRV_SECTOR_SIZE)
  . EXT_CSD_HC_ERASE_GRP_SIZE (4 -> 1)
  . EXT_CSD_HC_WP_GRP_SIZE (4 -> 1)
  . EXT_CSD_S_C_VCC[Q] (8 -> 1)
  . EXT_CSD_S_A_TIMEOUT (17 -> 1)
  . EXT_CSD_CARD_TYPE (7 -> 3)
      Dual data rate -> High-Speed mode

- Update:
  . EXT_CSD_CARD_TYPE (7 -> 3)
      High-Speed MultiMediaCard @ 26MHz & 52MHz
  . Performances (0xa -> 0x46)
      Class B at 3MB/s. -> Class J at 21MB/s
  . EXT_CSD_REV (5 -> 3)
      Rev 1.5 (spec v4.41) -> Rev 1.3 (spec v4.3)

- Use load/store API to set EXT_CSD_SEC_CNT

- Remove R/W keys, normally zeroed at reset
  . EXT_CSD_BOOT_INFO

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 50 ++++++++++++++++++--------------------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index df0e2345c0..2a687977d1 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -484,43 +484,29 @@ static void mmc_set_ext_csd(SDState *sd, uint64_t size)
 
     memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
 
+    /* Properties segment (RO) */
     sd->ext_csd[EXT_CSD_S_CMD_SET] = 0b1; /* supported command sets */
-    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
-    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations */
-    sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
-    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
-    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */
-    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
+    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x0; /* Boot information */
                                      /* Boot partition size. 128KB unit */
     sd->ext_csd[EXT_CSD_BOOT_MULT] = sd->boot_part_size / (128 * KiB);
-    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
-    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
+    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x1; /* Access size */
+    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x01; /* HC Erase unit size */
     sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x01; /* HC erase timeout */
     sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
-    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
-    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
-    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
-    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
-    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
-    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
-    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
-    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
-    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
-    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
-    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
-    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
-    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
-    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
-    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1;
-    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1;
-    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0x7;
-    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2;
-    sd->ext_csd[EXT_CSD_REV] = 0x5;
-    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
-    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3;
-    sd->ext_csd[159] = 0x00; /* Max enhanced area size */
-    sd->ext_csd[158] = 0x00; /* ... */
-    sd->ext_csd[157] = 0xEC; /* ... */
+    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x01; /* HC write protect group size */
+    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x01; /* Sleep current VCC  */
+    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x01; /* Sleep current VCCQ */
+    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x01; /* Sleep/Awake timeout */
+    stl_le_p(&sd->ext_csd[EXT_CSD_SEC_CNT], sectcount); /* Sector count */
+    sd->ext_csd[210] = 0x46; /* Min write perf for 8bit@52Mhz */
+    sd->ext_csd[209] = 0x46; /* Min read perf for 8bit@52Mhz  */
+    sd->ext_csd[208] = 0x46; /* Min write perf for 4bit@52Mhz */
+    sd->ext_csd[207] = 0x46; /* Min read perf for 4bit@52Mhz */
+    sd->ext_csd[206] = 0x46; /* Min write perf for 4bit@26Mhz */
+    sd->ext_csd[205] = 0x46; /* Min read perf for 4bit@26Mhz */
+    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0b11;
+    sd->ext_csd[EXT_CSD_STRUCTURE] = 2;
+    sd->ext_csd[EXT_CSD_REV] = 3;
 }
 
 static void emmc_set_csd(SDState *sd, uint64_t size)
-- 
2.41.0


Re: [PATCH v47 12/19] hw/sd/sdcard: Simplify EXT_CSD values for spec v4.3
Posted by Cédric Le Goater 4 months, 2 weeks ago
On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
> - Set some keys to not defined / implemented:
>    . EXT_CSD_HPI_FEATURES
>    . EXT_CSD_BKOPS_SUPPORT
>    . EXT_CSD_SEC_FEATURE_SUPPORT
>    . EXT_CSD_ERASE_TIMEOUT_MULT
>    . EXT_CSD_PART_SWITCH_TIME
>    . EXT_CSD_OUT_OF_INTERRUPT_TIME
> 
> - Simplify:
>    . EXT_CSD_ACC_SIZE (6 -> 1)
>        16KB of super_page_size -> 512B (BDRV_SECTOR_SIZE)
>    . EXT_CSD_HC_ERASE_GRP_SIZE (4 -> 1)
>    . EXT_CSD_HC_WP_GRP_SIZE (4 -> 1)
>    . EXT_CSD_S_C_VCC[Q] (8 -> 1)
>    . EXT_CSD_S_A_TIMEOUT (17 -> 1)
>    . EXT_CSD_CARD_TYPE (7 -> 3)
>        Dual data rate -> High-Speed mode
> 
> - Update:
>    . EXT_CSD_CARD_TYPE (7 -> 3)
>        High-Speed MultiMediaCard @ 26MHz & 52MHz
>    . Performances (0xa -> 0x46)
>        Class B at 3MB/s. -> Class J at 21MB/s
>    . EXT_CSD_REV (5 -> 3)
>        Rev 1.5 (spec v4.41) -> Rev 1.3 (spec v4.3)
> 
> - Use load/store API to set EXT_CSD_SEC_CNT
> 
> - Remove R/W keys, normally zeroed at reset
>    . EXT_CSD_BOOT_INFO
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

This should be merged in patch 10.

Thanks,

C.



> ---
>   hw/sd/sd.c | 50 ++++++++++++++++++--------------------------------
>   1 file changed, 18 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index df0e2345c0..2a687977d1 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -484,43 +484,29 @@ static void mmc_set_ext_csd(SDState *sd, uint64_t size)
>   
>       memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
>   
> +    /* Properties segment (RO) */
>       sd->ext_csd[EXT_CSD_S_CMD_SET] = 0b1; /* supported command sets */
> -    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
> -    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations */
> -    sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
> -    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
> -    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */
> -    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
> +    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x0; /* Boot information */
>                                        /* Boot partition size. 128KB unit */
>       sd->ext_csd[EXT_CSD_BOOT_MULT] = sd->boot_part_size / (128 * KiB);
> -    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
> -    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
> +    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x1; /* Access size */
> +    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x01; /* HC Erase unit size */
>       sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x01; /* HC erase timeout */
>       sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
> -    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
> -    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
> -    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
> -    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
> -    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
> -    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
> -    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
> -    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
> -    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
> -    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
> -    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
> -    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
> -    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
> -    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
> -    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1;
> -    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1;
> -    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0x7;
> -    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2;
> -    sd->ext_csd[EXT_CSD_REV] = 0x5;
> -    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
> -    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3;
> -    sd->ext_csd[159] = 0x00; /* Max enhanced area size */
> -    sd->ext_csd[158] = 0x00; /* ... */
> -    sd->ext_csd[157] = 0xEC; /* ... */
> +    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x01; /* HC write protect group size */
> +    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x01; /* Sleep current VCC  */
> +    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x01; /* Sleep current VCCQ */
> +    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x01; /* Sleep/Awake timeout */
> +    stl_le_p(&sd->ext_csd[EXT_CSD_SEC_CNT], sectcount); /* Sector count */
> +    sd->ext_csd[210] = 0x46; /* Min write perf for 8bit@52Mhz */
> +    sd->ext_csd[209] = 0x46; /* Min read perf for 8bit@52Mhz  */
> +    sd->ext_csd[208] = 0x46; /* Min write perf for 4bit@52Mhz */
> +    sd->ext_csd[207] = 0x46; /* Min read perf for 4bit@52Mhz */
> +    sd->ext_csd[206] = 0x46; /* Min write perf for 4bit@26Mhz */
> +    sd->ext_csd[205] = 0x46; /* Min read perf for 4bit@26Mhz */
> +    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0b11;
> +    sd->ext_csd[EXT_CSD_STRUCTURE] = 2;
> +    sd->ext_csd[EXT_CSD_REV] = 3;
>   }
>   
>   static void emmc_set_csd(SDState *sd, uint64_t size)


Re: [PATCH v47 12/19] hw/sd/sdcard: Simplify EXT_CSD values for spec v4.3
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
On 9/7/24 17:43, Cédric Le Goater wrote:
> On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
>> - Set some keys to not defined / implemented:
>>    . EXT_CSD_HPI_FEATURES
>>    . EXT_CSD_BKOPS_SUPPORT
>>    . EXT_CSD_SEC_FEATURE_SUPPORT
>>    . EXT_CSD_ERASE_TIMEOUT_MULT
>>    . EXT_CSD_PART_SWITCH_TIME
>>    . EXT_CSD_OUT_OF_INTERRUPT_TIME
>>
>> - Simplify:
>>    . EXT_CSD_ACC_SIZE (6 -> 1)
>>        16KB of super_page_size -> 512B (BDRV_SECTOR_SIZE)
>>    . EXT_CSD_HC_ERASE_GRP_SIZE (4 -> 1)
>>    . EXT_CSD_HC_WP_GRP_SIZE (4 -> 1)
>>    . EXT_CSD_S_C_VCC[Q] (8 -> 1)
>>    . EXT_CSD_S_A_TIMEOUT (17 -> 1)
>>    . EXT_CSD_CARD_TYPE (7 -> 3)
>>        Dual data rate -> High-Speed mode
>>
>> - Update:
>>    . EXT_CSD_CARD_TYPE (7 -> 3)
>>        High-Speed MultiMediaCard @ 26MHz & 52MHz
>>    . Performances (0xa -> 0x46)
>>        Class B at 3MB/s. -> Class J at 21MB/s
>>    . EXT_CSD_REV (5 -> 3)
>>        Rev 1.5 (spec v4.41) -> Rev 1.3 (spec v4.3)
>>
>> - Use load/store API to set EXT_CSD_SEC_CNT
>>
>> - Remove R/W keys, normally zeroed at reset
>>    . EXT_CSD_BOOT_INFO
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> This should be merged in patch 10.

OK.


[PATCH v47 13/19] hw/sd/sdcard: Migrate ExtCSD 'modes' register
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
Spec v4.3, chapter 8.4 "Extended CSD register":

  The Extended CSD register defines the card properties
  and selected modes. It is 512 bytes long. The most
  significant 320 bytes are the Properties segment, which
  defines the card capabilities and cannot be modified by
  the host. The lower 192 bytes are the Modes segment,
  which defines the configuration the card is working in.

Only migrate the Modes segment (192 lower bytes).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2a687977d1..a391f12b2a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -124,7 +124,13 @@ struct SDState {
     uint16_t rca;
     uint32_t card_status;
     uint8_t sd_status[64];
-    uint8_t ext_csd[512];
+    union {
+        uint8_t ext_csd[512];
+        struct {
+            uint8_t ext_csd_rw[192]; /* Modes segment */
+            uint8_t ext_csd_ro[320]; /* Properties segment */
+        };
+    };
 
     /* Static properties */
 
@@ -881,6 +887,24 @@ static const VMStateDescription sd_ocr_vmstate = {
     },
 };
 
+static bool vmstate_needed_for_emmc(void *opaque)
+{
+    SDState *sd = opaque;
+
+    return sd_is_emmc(sd);
+}
+
+static const VMStateDescription emmc_extcsd_vmstate = {
+    .name = "sd-card/ext_csd_modes-state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = vmstate_needed_for_emmc,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINT8_ARRAY(ext_csd_rw, SDState, 192),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 static int sd_vmstate_pre_load(void *opaque)
 {
     SDState *sd = opaque;
@@ -928,6 +952,7 @@ static const VMStateDescription sd_vmstate = {
     },
     .subsections = (const VMStateDescription * const []) {
         &sd_ocr_vmstate,
+        &emmc_extcsd_vmstate,
         NULL
     },
 };
-- 
2.41.0


[PATCH v47 14/19] hw/sd/sdcard: Add mmc SWITCH function support (CMD6)
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

switch operation in mmc cards, updated the ext_csd register to
request changes in card operations. Here we implement similar
sequence but requests are mostly dummy and make no change.

Implement SWITCH_ERROR if the write operation offset goes beyond
length of ext_csd.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c         | 56 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/sd/trace-events |  2 ++
 2 files changed, 58 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a391f12b2a..beb8e2730a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -642,6 +642,7 @@ static bool sd_req_rca_same(SDState *s, SDRequest req)
 FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
 FIELD(CSR, APP_CMD,                     5,  1)
 FIELD(CSR, FX_EVENT,                    6,  1)
+FIELD(CSR, SWITCH_ERROR,                7,  1)
 FIELD(CSR, READY_FOR_DATA,              8,  1)
 FIELD(CSR, CURRENT_STATE,               9,  4)
 FIELD(CSR, ERASE_RESET,                13,  1)
@@ -1091,6 +1092,47 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
     return ret;
 }
 
+enum ExtCsdAccessMode {
+    EXT_CSD_ACCESS_MODE_COMMAND_SET = 0,
+    EXT_CSD_ACCESS_MODE_SET_BITS    = 1,
+    EXT_CSD_ACCESS_MODE_CLEAR_BITS  = 2,
+    EXT_CSD_ACCESS_MODE_WRITE_BYTE  = 3
+};
+
+static void mmc_function_switch(SDState *sd, uint32_t arg)
+{
+    uint8_t access = extract32(arg, 24, 2);
+    uint8_t index = extract32(arg, 16, 8);
+    uint8_t value = extract32(arg, 8, 8);
+    uint8_t b = sd->ext_csd[index];
+
+    trace_sdcard_switch(access, index, value, extract32(arg, 0, 2));
+
+    if (index >= 192) {
+        qemu_log_mask(LOG_GUEST_ERROR, "MMC switching illegal offset\n");
+        sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
+        return;
+    }
+
+    switch (access) {
+    case EXT_CSD_ACCESS_MODE_COMMAND_SET:
+        qemu_log_mask(LOG_UNIMP, "MMC Command set switching not supported\n");
+        return;
+    case EXT_CSD_ACCESS_MODE_SET_BITS:
+        b |= value;
+        break;
+    case EXT_CSD_ACCESS_MODE_CLEAR_BITS:
+        b &= ~value;
+        break;
+    case EXT_CSD_ACCESS_MODE_WRITE_BYTE:
+        b = value;
+        break;
+    }
+
+    trace_sdcard_ext_csd_update(index, sd->ext_csd[index], b);
+    sd->ext_csd[index] = b;
+}
+
 static void sd_function_switch(SDState *sd, uint32_t arg)
 {
     int i, mode, new_func;
@@ -1402,6 +1444,19 @@ static sd_rsp_type_t sd_cmd_SWITCH_FUNCTION(SDState *sd, SDRequest req)
     return sd_cmd_to_sendingdata(sd, req, 0, NULL, 64);
 }
 
+static sd_rsp_type_t emmc_cmd_SWITCH(SDState *sd, SDRequest req)
+{
+    switch (sd->state) {
+    case sd_transfer_state:
+        sd->state = sd_programming_state;
+        mmc_function_switch(sd, req.arg);
+        sd->state = sd_transfer_state;
+        return sd_r1b;
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 /* CMD7 */
 static sd_rsp_type_t sd_cmd_DE_SELECT_CARD(SDState *sd, SDRequest req)
 {
@@ -2581,6 +2636,7 @@ static const SDProto sd_proto_emmc = {
         [3]  = {0,  sd_ac,   "SET_RELATIVE_ADDR", emmc_cmd_SET_RELATIVE_ADDR},
         [4]  = {0,  sd_bc,   "SEND_DSR", sd_cmd_unimplemented},
         [5]  = {0,  sd_ac,   "SLEEP/AWAKE", emmc_cmd_sleep_awake},
+        [6]  = {10, sd_adtc, "SWITCH", emmc_cmd_SWITCH},
         [7]  = {0,  sd_ac,   "(DE)SELECT_CARD", sd_cmd_DE_SELECT_CARD},
         [8]  = {0,  sd_adtc, "SEND_EXT_CSD", emmc_cmd_SEND_EXT_CSD},
         [9]  = {0,  sd_ac,   "SEND_CSD", sd_cmd_SEND_CSD},
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 5dfe6be7b7..43671dc791 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -57,6 +57,8 @@ sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t offset, uint8_t value) "%s %20s/ CMD%02d ofs %"PRIu32" value 0x%02x"
 sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t offset, uint64_t size, uint32_t blklen) "%s %20s/ CMD%02d ofs %"PRIu32" size %"PRIu64" blklen %" PRIu32
 sdcard_set_voltage(uint16_t millivolts) "%u mV"
+sdcard_ext_csd_update(unsigned index, uint8_t oval, uint8_t nval) "index %u: 0x%02x -> 0x%02x"
+sdcard_switch(unsigned access, unsigned index, unsigned value, unsigned set) "SWITCH acc:%u idx:%u val:%u set:%u"
 
 # pxa2xx_mmci.c
 pxa2xx_mmci_read(uint8_t size, uint32_t addr, uint32_t value) "size %d addr 0x%02x value 0x%08x"
-- 
2.41.0


[PATCH v47 15/19] hw/sd/sdcard: Implement eMMC 'boot-mode'
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
Spec v4.3 chapter 7.2.2 "Boot operation":

  If the CMD line is held LOW for 74 clock cycles and more
  after power-up before the first command is issued, the
  slave recognizes that boot mode is being initiated and
  starts preparing boot data internally.

Track uptime since last reset, add the sd_uptime_ns() helper.

When the first command is received, check at least 74 clocks
are elapsed (during the identification phase, at a 10kHz rate)
then enable BOOT_MODE in the Ext_CSD register.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c         | 38 ++++++++++++++++++++++++++++++++++++++
 hw/sd/trace-events |  1 +
 2 files changed, 39 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index beb8e2730a..c7f8ea11c1 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -163,6 +163,8 @@ struct SDState {
      */
     bool expecting_acmd;
     uint32_t blk_written;
+    int64_t reset_time_ns;
+    uint32_t cmd_count;
 
     uint64_t data_start;
     uint32_t data_offset;
@@ -352,6 +354,11 @@ static uint8_t sd_crc7(const void *message, size_t width)
     return shift_reg;
 }
 
+static int64_t sd_uptime_ns(SDState *sd)
+{
+    return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - sd->reset_time_ns;
+}
+
 /* Operation Conditions register */
 
 #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
@@ -479,6 +486,10 @@ static void emmc_set_cid(SDState *sd)
 #define CMULT_SHIFT     9        /* 512 times HWBLOCK_SIZE */
 #define WPGROUP_SIZE    (1 << (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT))
 
+#define OD_FREQ_MIN_HZ   10000
+#define OD_FREQ_MAX_HZ  400000
+#define BOOT_MODE_DELAY_CYCLES_MIN 74
+
 static const uint8_t sd_csd_rw_mask[16] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xfc, 0xfe,
@@ -798,6 +809,8 @@ static void sd_reset(DeviceState *dev)
 
     sect = sd_addr_to_wpnum(size) + 1;
 
+    sd->reset_time_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    sd->cmd_count = 0;
     sd->state = sd_idle_state;
 
     /* card registers */
@@ -906,6 +919,18 @@ static const VMStateDescription emmc_extcsd_vmstate = {
     },
 };
 
+static const VMStateDescription sdmmc_uptime_cmdcnt_vmstate = {
+    .name = "sd-card/uptime-command_count-state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = vmstate_needed_for_emmc,
+    .fields = (const VMStateField[]) {
+        VMSTATE_INT64(reset_time_ns, SDState),
+        VMSTATE_UINT32(cmd_count, SDState),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 static int sd_vmstate_pre_load(void *opaque)
 {
     SDState *sd = opaque;
@@ -954,6 +979,7 @@ static const VMStateDescription sd_vmstate = {
     .subsections = (const VMStateDescription * const []) {
         &sd_ocr_vmstate,
         &emmc_extcsd_vmstate,
+        &sdmmc_uptime_cmdcnt_vmstate,
         NULL
     },
 };
@@ -1980,6 +2006,16 @@ static sd_rsp_type_t sd_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
         sd->state = sd_ready_state;
     }
 
+    if (sd_is_emmc(sd) && sd->cmd_count == 1) {
+        int64_t clk_cycles = sd_uptime_ns(sd) / OD_FREQ_MIN_HZ;
+
+        trace_sdcard_ext_csd_bootmode(sd_uptime_ns(sd), clk_cycles,
+                                      clk_cycles > BOOT_MODE_DELAY_CYCLES_MIN);
+        if (clk_cycles > BOOT_MODE_DELAY_CYCLES_MIN) {
+            sd->ext_csd[EXT_CSD_PART_CONFIG] |= (1 << 3);
+        }
+    }
+
     return sd_r3;
 }
 
@@ -2162,6 +2198,8 @@ int sd_do_command(SDState *sd, SDRequest *req,
         return 0;
     }
 
+    ++sd->cmd_count;
+
     if (sd->state == sd_inactive_state) {
         rtype = sd_illegal;
         goto send_response;
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 43671dc791..5454e55077 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -57,6 +57,7 @@ sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t offset, uint8_t value) "%s %20s/ CMD%02d ofs %"PRIu32" value 0x%02x"
 sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t offset, uint64_t size, uint32_t blklen) "%s %20s/ CMD%02d ofs %"PRIu32" size %"PRIu64" blklen %" PRIu32
 sdcard_set_voltage(uint16_t millivolts) "%u mV"
+sdcard_ext_csd_bootmode(int64_t uptime_ns, int64_t clk_cycles, unsigned enabled) "%"PRId64" ns, %"PRId64" cycles, boot mode: %u"
 sdcard_ext_csd_update(unsigned index, uint8_t oval, uint8_t nval) "index %u: 0x%02x -> 0x%02x"
 sdcard_switch(unsigned access, unsigned index, unsigned value, unsigned set) "SWITCH acc:%u idx:%u val:%u set:%u"
 
-- 
2.41.0


[Aspeed PATCH v47 16/19] hw/sd/sdcard: Support boot area in emmc image
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
From: Joel Stanley <joel@jms.id.au>

This assumes a specially constructed image:

  dd if=/dev/zero of=mmc-bootarea.img count=2 bs=1M
  dd if=u-boot-spl.bin of=mmc-bootarea.img conv=notrunc
  dd if=u-boot.bin of=mmc-bootarea.img conv=notrunc count=64 bs=1K
  cat mmc-bootarea.img obmc-phosphor-image.wic > mmc.img
  truncate --size 16GB mmc.img
  truncate --size 128MB mmc-bootarea.img

For now this still requires a mtd image to load the SPL:

  qemu-system-arm -M tacoma-bmc -nographic \
   -global driver=sd-card,property=emmc,value=true \
   -drive file=mmc.img,if=sd,index=2 \
   -drive file=mmc-bootarea.img,if=mtd,format=raw

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
TODO: Update QEMU command in description
---
 include/hw/sd/sd.h |  1 +
 hw/sd/sd.c         | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index d35a839f5e..07435d2e17 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -132,6 +132,7 @@ struct SDCardClass {
     bool (*get_readonly)(SDState *sd);
     void (*set_cid)(SDState *sd);
     void (*set_csd)(SDState *sd, uint64_t size);
+    uint32_t (*bootpart_offset)(SDState *sd);
 
     const struct SDProto *proto;
 };
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index c7f8ea11c1..5830725629 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -774,6 +774,13 @@ static uint32_t sd_blk_len(SDState *sd)
     return sd->blk_len;
 }
 
+static uint32_t sd_bootpart_offset(SDState *sd)
+{
+    SDCardClass *sc = SDMMC_COMMON_GET_CLASS(sd);
+
+    return sc->bootpart_offset ? sc->bootpart_offset(sd) : 0;
+}
+
 static uint64_t sd_req_get_address(SDState *sd, SDRequest req)
 {
     uint64_t addr;
@@ -1026,9 +1033,33 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
     qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0);
 }
 
+/*
+ * This requires a disk image that has two boot partitions inserted at the
+ * beginning of it. The size of the boot partitions are configured in the
+ * ext_csd structure, which is hardcoded in qemu. They are currently set to
+ * 1MB each.
+ */
+static uint32_t emmc_bootpart_offset(SDState *sd)
+{
+    unsigned int access = sd->ext_csd[EXT_CSD_PART_CONFIG]
+                          & EXT_CSD_PART_CONFIG_ACC_MASK;
+
+    switch (access) {
+    case EXT_CSD_PART_CONFIG_ACC_DEFAULT:
+        return sd->boot_part_size * 2;
+    case EXT_CSD_PART_CONFIG_ACC_BOOT0:
+        return 0;
+    case EXT_CSD_PART_CONFIG_ACC_BOOT0 + 1:
+        return sd->boot_part_size * 1;
+    default:
+         g_assert_not_reached();
+    }
+}
+
 static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 {
     trace_sdcard_read_block(addr, len);
+    addr += sd_bootpart_offset(sd);
     if (!sd->blk || blk_pread(sd->blk, addr, len, sd->data, 0) < 0) {
         fprintf(stderr, "sd_blk_read: read error on host side\n");
     }
@@ -1037,6 +1068,7 @@ static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 {
     trace_sdcard_write_block(addr, len);
+    addr += sd_bootpart_offset(sd);
     if (!sd->blk || blk_pwrite(sd->blk, addr, len, sd->data, 0) < 0) {
         fprintf(stderr, "sd_blk_write: write error on host side\n");
     }
@@ -2871,6 +2903,7 @@ static void emmc_class_init(ObjectClass *klass, void *data)
 
     sc->set_cid = emmc_set_cid;
     sc->set_csd = emmc_set_csd;
+    sc->bootpart_offset = emmc_bootpart_offset;
 }
 
 static const TypeInfo sd_types[] = {
-- 
2.41.0


Re: [Aspeed PATCH v47 16/19] hw/sd/sdcard: Support boot area in emmc image
Posted by Cédric Le Goater 4 months, 2 weeks ago
On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
> From: Joel Stanley <joel@jms.id.au>
> 
> This assumes a specially constructed image:
> 
>    dd if=/dev/zero of=mmc-bootarea.img count=2 bs=1M
>    dd if=u-boot-spl.bin of=mmc-bootarea.img conv=notrunc
>    dd if=u-boot.bin of=mmc-bootarea.img conv=notrunc count=64 bs=1K
>    cat mmc-bootarea.img obmc-phosphor-image.wic > mmc.img
>    truncate --size 16GB mmc.img
>    truncate --size 128MB mmc-bootarea.img
> 
> For now this still requires a mtd image to load the SPL:
> 
>    qemu-system-arm -M tacoma-bmc -nographic \
>     -global driver=sd-card,property=emmc,value=true \
>     -drive file=mmc.img,if=sd,index=2 \
>     -drive file=mmc-bootarea.img,if=mtd,format=raw
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> TODO: Update QEMU command in description

hmm, this patch was also modified since last sent.


Thanks,

C.




> ---
>   include/hw/sd/sd.h |  1 +
>   hw/sd/sd.c         | 33 +++++++++++++++++++++++++++++++++
>   2 files changed, 34 insertions(+)
> 
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index d35a839f5e..07435d2e17 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -132,6 +132,7 @@ struct SDCardClass {
>       bool (*get_readonly)(SDState *sd);
>       void (*set_cid)(SDState *sd);
>       void (*set_csd)(SDState *sd, uint64_t size);
> +    uint32_t (*bootpart_offset)(SDState *sd);
>   
>       const struct SDProto *proto;
>   };
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index c7f8ea11c1..5830725629 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -774,6 +774,13 @@ static uint32_t sd_blk_len(SDState *sd)
>       return sd->blk_len;
>   }
>   
> +static uint32_t sd_bootpart_offset(SDState *sd)
> +{
> +    SDCardClass *sc = SDMMC_COMMON_GET_CLASS(sd);
> +
> +    return sc->bootpart_offset ? sc->bootpart_offset(sd) : 0;
> +}
> +
>   static uint64_t sd_req_get_address(SDState *sd, SDRequest req)
>   {
>       uint64_t addr;
> @@ -1026,9 +1033,33 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
>       qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0);
>   }
>   
> +/*
> + * This requires a disk image that has two boot partitions inserted at the
> + * beginning of it. The size of the boot partitions are configured in the
> + * ext_csd structure, which is hardcoded in qemu. They are currently set to
> + * 1MB each.
> + */
> +static uint32_t emmc_bootpart_offset(SDState *sd)
> +{
> +    unsigned int access = sd->ext_csd[EXT_CSD_PART_CONFIG]
> +                          & EXT_CSD_PART_CONFIG_ACC_MASK;
> +
> +    switch (access) {
> +    case EXT_CSD_PART_CONFIG_ACC_DEFAULT:
> +        return sd->boot_part_size * 2;
> +    case EXT_CSD_PART_CONFIG_ACC_BOOT0:
> +        return 0;
> +    case EXT_CSD_PART_CONFIG_ACC_BOOT0 + 1:
> +        return sd->boot_part_size * 1;
> +    default:
> +         g_assert_not_reached();
> +    }
> +}
> +
>   static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
>   {
>       trace_sdcard_read_block(addr, len);
> +    addr += sd_bootpart_offset(sd);
>       if (!sd->blk || blk_pread(sd->blk, addr, len, sd->data, 0) < 0) {
>           fprintf(stderr, "sd_blk_read: read error on host side\n");
>       }
> @@ -1037,6 +1068,7 @@ static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
>   static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
>   {
>       trace_sdcard_write_block(addr, len);
> +    addr += sd_bootpart_offset(sd);
>       if (!sd->blk || blk_pwrite(sd->blk, addr, len, sd->data, 0) < 0) {
>           fprintf(stderr, "sd_blk_write: write error on host side\n");
>       }
> @@ -2871,6 +2903,7 @@ static void emmc_class_init(ObjectClass *klass, void *data)
>   
>       sc->set_cid = emmc_set_cid;
>       sc->set_csd = emmc_set_csd;
> +    sc->bootpart_offset = emmc_bootpart_offset;
>   }
>   
>   static const TypeInfo sd_types[] = {


[Aspeed PATCH v47 17/19] hw/sd/sdcard: Subtract bootarea size from blk
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
From: Joel Stanley <joel@jms.id.au>

The userdata size is derived from the file the user passes on the
command line, but we must take into account the boot areas.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Squash in previous?
---
 hw/sd/sd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 5830725629..291497468f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -814,6 +814,10 @@ static void sd_reset(DeviceState *dev)
     }
     size = sect << HWBLOCK_SHIFT;
 
+    if (sc->bootpart_offset) {
+        size -= sd_bootpart_offset(sd);
+    }
+
     sect = sd_addr_to_wpnum(size) + 1;
 
     sd->reset_time_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-- 
2.41.0


[Aspeed PATCH v47 18/19] hw/sd/sdcard: Add boot config support
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
From: Joel Stanley <joel@jms.id.au>

With this correctly set we can use the enable bit to detect if
partition support is enabled.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Also squash?
---
 hw/sd/sd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 291497468f..6aa83251f7 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1047,6 +1047,12 @@ static uint32_t emmc_bootpart_offset(SDState *sd)
 {
     unsigned int access = sd->ext_csd[EXT_CSD_PART_CONFIG]
                           & EXT_CSD_PART_CONFIG_ACC_MASK;
+    unsigned int enable = sd->ext_csd[EXT_CSD_PART_CONFIG]
+                          & EXT_CSD_PART_CONFIG_EN_MASK;
+
+    if (!enable) {
+        return 0;
+    }
 
     switch (access) {
     case EXT_CSD_PART_CONFIG_ACC_DEFAULT:
-- 
2.41.0


Re: [Aspeed PATCH v47 18/19] hw/sd/sdcard: Add boot config support
Posted by Cédric Le Goater 4 months, 2 weeks ago
On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
> From: Joel Stanley <joel@jms.id.au>
> 
> With this correctly set we can use the enable bit to detect if
> partition support is enabled.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Also squash?

where is the "boot-config" property gone ?


Thanks,

C.


> ---
>   hw/sd/sd.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 291497468f..6aa83251f7 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1047,6 +1047,12 @@ static uint32_t emmc_bootpart_offset(SDState *sd)
>   {
>       unsigned int access = sd->ext_csd[EXT_CSD_PART_CONFIG]
>                             & EXT_CSD_PART_CONFIG_ACC_MASK;
> +    unsigned int enable = sd->ext_csd[EXT_CSD_PART_CONFIG]
> +                          & EXT_CSD_PART_CONFIG_EN_MASK;
> +
> +    if (!enable) {
> +        return 0;
> +    }
>   
>       switch (access) {
>       case EXT_CSD_PART_CONFIG_ACC_DEFAULT:


Re: [Aspeed PATCH v47 18/19] hw/sd/sdcard: Add boot config support
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
On 9/7/24 17:52, Cédric Le Goater wrote:
> On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
>> From: Joel Stanley <joel@jms.id.au>
>>
>> With this correctly set we can use the enable bit to detect if
>> partition support is enabled.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> Also squash?
> 
> where is the "boot-config" property gone ?

Replaced by patch #15 "Implement eMMC 'boot-mode'".

[PATCH v47 19/19] hw/sd/sdcard: Enable TYPE_EMMC card model
Posted by Philippe Mathieu-Daudé 4 months, 2 weeks ago
Now than the implementation is functional, allow
to instantiate it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sd/sd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6aa83251f7..4a6e9cc035 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2941,7 +2941,6 @@ static const TypeInfo sd_types[] = {
         .name           = TYPE_EMMC,
         .parent         = TYPE_SDMMC_COMMON,
         .class_init     = emmc_class_init,
-        .abstract       = true, /* FIXME: Remove once model fully functional */
     },
 };
 
-- 
2.41.0


Re: [PATCH v47 19/19] hw/sd/sdcard: Enable TYPE_EMMC card model
Posted by Cédric Le Goater 4 months, 2 weeks ago
On 7/9/24 5:25 PM, Philippe Mathieu-Daudé wrote:
> Now than the implementation is functional, allow
> to instantiate it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

I don't think this is necessary. emmc should be functional once patch 1-15
are merged. The boot part is an extension.

Thanks,

C.



> ---
>   hw/sd/sd.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 6aa83251f7..4a6e9cc035 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -2941,7 +2941,6 @@ static const TypeInfo sd_types[] = {
>           .name           = TYPE_EMMC,
>           .parent         = TYPE_SDMMC_COMMON,
>           .class_init     = emmc_class_init,
> -        .abstract       = true, /* FIXME: Remove once model fully functional */
>       },
>   };
>