[PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Jan Luebbe]

For testing eMMC-specific functionality (such as handling boot
partitions), it would be very useful to attach them to generic VMs such
as x86_64 via the sdhci-pci device:
 ...
 -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
 -device sdhci-pci \
 -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
 ...

While most eMMCs are soldered to boards, they can also be connected to
SD controllers with just a passive adapter, such as:
 https://docs.radxa.com/en/accessories/emmc-to-usd
 https://github.com/voltlog/emmc-wfbga153-microsd

The only change necessary to make the options above work is to avoid
disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
kernel already supports this just fine.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 hw/sd/sd.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a5d2d929a8af..2d3467c3d956 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2865,8 +2865,6 @@ static void emmc_class_init(ObjectClass *klass, void *data)
     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;
 
-- 
2.39.5

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Philippe Mathieu-Daudé]

On 15/10/24 10:56, Jan Luebbe wrote:
> For testing eMMC-specific functionality (such as handling boot
> partitions), it would be very useful to attach them to generic VMs such
> as x86_64 via the sdhci-pci device:
>   ...
>   -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
>   -device sdhci-pci \
>   -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
>   ...
> 
> While most eMMCs are soldered to boards, they can also be connected to
> SD controllers with just a passive adapter, such as:
>   https://docs.radxa.com/en/accessories/emmc-to-usd
>   https://github.com/voltlog/emmc-wfbga153-microsd
> 
> The only change necessary to make the options above work is to avoid
> disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
> kernel already supports this just fine.
> 
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> ---
>   hw/sd/sd.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index a5d2d929a8af..2d3467c3d956 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -2865,8 +2865,6 @@ static void emmc_class_init(ObjectClass *klass, void *data)
>       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;
>   

Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Cédric Le Goater]

On 10/15/24 17:00, Philippe Mathieu-Daudé wrote:
> On 15/10/24 10:56, Jan Luebbe wrote:
>> For testing eMMC-specific functionality (such as handling boot
>> partitions), it would be very useful to attach them to generic VMs such
>> as x86_64 via the sdhci-pci device:
>>   ...
>>   -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
>>   -device sdhci-pci \
>>   -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
>>   ...
>>
>> While most eMMCs are soldered to boards, they can also be connected to
>> SD controllers with just a passive adapter, such as:
>>   https://docs.radxa.com/en/accessories/emmc-to-usd
>>   https://github.com/voltlog/emmc-wfbga153-microsd
>>
>> The only change necessary to make the options above work is to avoid
>> disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
>> kernel already supports this just fine.
>>
>> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>

Nice !

Would it be possible to add an avocado test ?

Thanks,

C.

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Daniel P. Berrangé]

On Tue, Oct 15, 2024 at 05:17:26PM +0200, Cédric Le Goater wrote:
> On 10/15/24 17:00, Philippe Mathieu-Daudé wrote:
> > On 15/10/24 10:56, Jan Luebbe wrote:
> > > For testing eMMC-specific functionality (such as handling boot
> > > partitions), it would be very useful to attach them to generic VMs such
> > > as x86_64 via the sdhci-pci device:
> > >   ...
> > >   -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
> > >   -device sdhci-pci \
> > >   -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
> > >   ...
> > > 
> > > While most eMMCs are soldered to boards, they can also be connected to
> > > SD controllers with just a passive adapter, such as:
> > >   https://docs.radxa.com/en/accessories/emmc-to-usd
> > >   https://github.com/voltlog/emmc-wfbga153-microsd
> > > 
> > > The only change necessary to make the options above work is to avoid
> > > disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
> > > kernel already supports this just fine.
> > > 
> > > Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> 
> Nice !
> 
> Would it be possible to add an avocado test ?

NB, no new avocado tests please. Only use the recently introduced
'functional' tests framework for new tests.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Cédric Le Goater]

On 10/15/24 17:20, Daniel P. Berrangé wrote:
> On Tue, Oct 15, 2024 at 05:17:26PM +0200, Cédric Le Goater wrote:
>> On 10/15/24 17:00, Philippe Mathieu-Daudé wrote:
>>> On 15/10/24 10:56, Jan Luebbe wrote:
>>>> For testing eMMC-specific functionality (such as handling boot
>>>> partitions), it would be very useful to attach them to generic VMs such
>>>> as x86_64 via the sdhci-pci device:
>>>>    ...
>>>>    -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
>>>>    -device sdhci-pci \
>>>>    -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
>>>>    ...
>>>>
>>>> While most eMMCs are soldered to boards, they can also be connected to
>>>> SD controllers with just a passive adapter, such as:
>>>>    https://docs.radxa.com/en/accessories/emmc-to-usd
>>>>    https://github.com/voltlog/emmc-wfbga153-microsd
>>>>
>>>> The only change necessary to make the options above work is to avoid
>>>> disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
>>>> kernel already supports this just fine.
>>>>
>>>> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
>>
>> Nice !
>>
>> Would it be possible to add an avocado test ?
> 
> NB, no new avocado tests please. Only use the recently introduced
> 'functional' tests framework for new tests.

True. That reminds me that the aspeed test file needs a conversion.

Thanks,

C.


> 
> 
> With regards,
> Daniel

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Peter Maydell]

On Tue, 15 Oct 2024 at 14:57, Jan Luebbe <jlu@pengutronix.de> wrote:
>
> For testing eMMC-specific functionality (such as handling boot
> partitions), it would be very useful to attach them to generic VMs such
> as x86_64 via the sdhci-pci device:
>  ...
>  -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
>  -device sdhci-pci \
>  -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
>  ...
>
> While most eMMCs are soldered to boards, they can also be connected to
> SD controllers with just a passive adapter, such as:
>  https://docs.radxa.com/en/accessories/emmc-to-usd
>  https://github.com/voltlog/emmc-wfbga153-microsd
>
> The only change necessary to make the options above work is to avoid
> disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
> kernel already supports this just fine.
>
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>

Applied to target-arm.next, thanks (unless anybody would
prefer it to go via some other route).

-- PMM

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Philippe Mathieu-Daudé]

On 18/10/24 12:42, Peter Maydell wrote:
> On Tue, 15 Oct 2024 at 14:57, Jan Luebbe <jlu@pengutronix.de> wrote:
>>
>> For testing eMMC-specific functionality (such as handling boot
>> partitions), it would be very useful to attach them to generic VMs such
>> as x86_64 via the sdhci-pci device:
>>   ...
>>   -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
>>   -device sdhci-pci \
>>   -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
>>   ...
>>
>> While most eMMCs are soldered to boards, they can also be connected to
>> SD controllers with just a passive adapter, such as:
>>   https://docs.radxa.com/en/accessories/emmc-to-usd
>>   https://github.com/voltlog/emmc-wfbga153-microsd
>>
>> The only change necessary to make the options above work is to avoid
>> disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
>> kernel already supports this just fine.
>>
>> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> 
> Applied to target-arm.next, thanks (unless anybody would
> prefer it to go via some other route).

Thanks!

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Peter Maydell]

On Fri, 18 Oct 2024 at 16:42, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 15 Oct 2024 at 14:57, Jan Luebbe <jlu@pengutronix.de> wrote:
> >
> > For testing eMMC-specific functionality (such as handling boot
> > partitions), it would be very useful to attach them to generic VMs such
> > as x86_64 via the sdhci-pci device:
> >  ...
> >  -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
> >  -device sdhci-pci \
> >  -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
> >  ...
> >
> > While most eMMCs are soldered to boards, they can also be connected to
> > SD controllers with just a passive adapter, such as:
> >  https://docs.radxa.com/en/accessories/emmc-to-usd
> >  https://github.com/voltlog/emmc-wfbga153-microsd
> >
> > The only change necessary to make the options above work is to avoid
> > disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
> > kernel already supports this just fine.
> >
> > Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
>
> Applied to target-arm.next, thanks (unless anybody would
> prefer it to go via some other route).

I'm dropping this from target-arm.next since it seems like
we have a problem with the handling of boot partitions
and how the user should provide an image for an emmc card
that has boot partitions). Since that's an emmc specific
thing, sorting that out with a minimum of breaking
compatibility with previously working setups is going to
be easier if we stay temporarily in the state of "emmc
only happens for the specific board that creates them
and the user can't arbitrarily create them on the
command line".

I expect this to just be a temporary delay while we sort
out in the other thread how emmc boot partitions should work.

thanks
-- PMM

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Jan Lübbe]

On Tue, 2024-10-29 at 15:06 +0000, Peter Maydell wrote:
> On Fri, 18 Oct 2024 at 16:42, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On Tue, 15 Oct 2024 at 14:57, Jan Luebbe <jlu@pengutronix.de> wrote:
> > > For testing eMMC-specific functionality (such as handling boot
> > > partitions), it would be very useful to attach them to generic VMs such
> > > as x86_64 via the sdhci-pci device:
> > >  ...
> > >  -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
> > >  -device sdhci-pci \
> > >  -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
> > >  ...
> > > 
> > > While most eMMCs are soldered to boards, they can also be connected to
> > > SD controllers with just a passive adapter, such as:
> > >  https://docs.radxa.com/en/accessories/emmc-to-usd
> > >  https://github.com/voltlog/emmc-wfbga153-microsd
> > > 
> > > The only change necessary to make the options above work is to avoid
> > > disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
> > > kernel already supports this just fine.
> > > 
> > > Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> > 
> > Applied to target-arm.next, thanks (unless anybody would
> > prefer it to go via some other route).
> 
> I'm dropping this from target-arm.next since it seems like
> we have a problem with the handling of boot partitions
> and how the user should provide an image for an emmc card
> that has boot partitions). Since that's an emmc specific
> thing, sorting that out with a minimum of breaking
> compatibility with previously working setups is going to
> be easier if we stay temporarily in the state of "emmc
> only happens for the specific board that creates them
> and the user can't arbitrarily create them on the
> command line".
> 
> I expect this to just be a temporary delay while we sort
> out in the other thread how emmc boot partitions should work.

With Cédric's e8f3acdbb8 ("aspeed: Don't set always boot properties of the emmc
device") and my c078298301 ("hw/sd/sdcard: Fix calculation of size when using
eMMC boot partitions") in master, compatibility for existing setups should be
taken care of.

As mentioned in Cédric's patch, allowing user creatable eMMC devices is still
desirable. With my patch, that would work for machines where the SD controller
is user-created as well (e.g. x86_64 with sdhci-pci).

For machines where the SD controller and SD/eMMC are pre-created, additional
changes seem to be needed. Would you consider taking this patch to solve the
simple case first?

Thanks,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] hw/sd/sdcard: Allow user creation of eMMCs

[Cédric Le Goater]

On 11/8/24 10:29, Jan Lübbe wrote:
> On Tue, 2024-10-29 at 15:06 +0000, Peter Maydell wrote:
>> On Fri, 18 Oct 2024 at 16:42, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On Tue, 15 Oct 2024 at 14:57, Jan Luebbe <jlu@pengutronix.de> wrote:
>>>> For testing eMMC-specific functionality (such as handling boot
>>>> partitions), it would be very useful to attach them to generic VMs such
>>>> as x86_64 via the sdhci-pci device:
>>>>   ...
>>>>   -drive if=none,id=emmc-drive,file=emmc.img,format=raw \
>>>>   -device sdhci-pci \
>>>>   -device emmc,id=emmc0,drive=emmc-drive,boot-partition-size=1048576 \
>>>>   ...
>>>>
>>>> While most eMMCs are soldered to boards, they can also be connected to
>>>> SD controllers with just a passive adapter, such as:
>>>>   https://docs.radxa.com/en/accessories/emmc-to-usd
>>>>   https://github.com/voltlog/emmc-wfbga153-microsd
>>>>
>>>> The only change necessary to make the options above work is to avoid
>>>> disabling user_creatable, so do that. The SDHCI-PCI driver in the Linux
>>>> kernel already supports this just fine.
>>>>
>>>> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
>>>
>>> Applied to target-arm.next, thanks (unless anybody would
>>> prefer it to go via some other route).
>>
>> I'm dropping this from target-arm.next since it seems like
>> we have a problem with the handling of boot partitions
>> and how the user should provide an image for an emmc card
>> that has boot partitions). Since that's an emmc specific
>> thing, sorting that out with a minimum of breaking
>> compatibility with previously working setups is going to
>> be easier if we stay temporarily in the state of "emmc
>> only happens for the specific board that creates them
>> and the user can't arbitrarily create them on the
>> command line".
>>
>> I expect this to just be a temporary delay while we sort
>> out in the other thread how emmc boot partitions should work.
> 
> With Cédric's e8f3acdbb8 ("aspeed: Don't set always boot properties of the emmc
> device") and my c078298301 ("hw/sd/sdcard: Fix calculation of size when using
> eMMC boot partitions") in master, compatibility for existing setups should be
> taken care of.
> 
> As mentioned in Cédric's patch, allowing user creatable eMMC devices is still
> desirable. With my patch, that would work for machines where the SD controller
> is user-created as well (e.g. x86_64 with sdhci-pci).
> 
> For machines where the SD controller and SD/eMMC are pre-created, additional
> changes seem to be needed.

Yes. That's a problem to fix for the emmc/sd devices created on the
Aspeed machines. They should only be created when defaults_enabled(),
just like the flash devices.

Also, to be able to assign a device to a sd bus from the command line,
more changes are required. A first series would be to avoid referencing
"sd-bus" where possible :

-        qdev_realize_and_unref(card,
-                               qdev_get_child_bus(DEVICE(sdhci), "sd-bus"),
-                               &error_fatal);
+        qdev_realize_and_unref(card, BUS(&sdhci->sdbus), &error_fatal);

the final touch would be :

  void sdhci_initfn(SDHCIState *s)
  {
-    qbus_init(&s->sdbus, sizeof(s->sdbus), TYPE_SDHCI_BUS, DEVICE(s), "sd-bus");
+    qbus_init(&s->sdbus, sizeof(s->sdbus), TYPE_SDHCI_BUS, DEVICE(s), NULL);

I haven't looked at all the machines though. Aspeed works fine with that
and emmc devices can be defined using :

   -blockdev node-name=emmc0,driver=file,filename=mmc-ast2600-evb-noboot.raw
   -device emmc,bus=sdhci-bus.2,drive=emmc0,id=foo

> Would you consider taking this patch to solve the
> simple case first?

I think we need to address the above first for sd and emmc devices.

Thanks,

C.