diff mbox series

[v2,1/6] efi_loader: add RAM disk device path

Message ID 20230714054406.761508-2-masahisa.kojima@linaro.org
State New
Headers show
Series introduce EFI_RAM_DISK_PROTOCOL | expand

Commit Message

Masahisa Kojima July 14, 2023, 5:44 a.m. UTC 
This is a preparation to add the EFI_RAM_DISK_PROTOCOL.
This commit adds the RAM disk device path structure
and text conversion to Device Path to Text Protocol.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
No update since v1

 include/efi_api.h                        | 19 +++++++++++++++++++
 lib/efi_loader/efi_device_path_to_text.c | 14 ++++++++++++++
 2 files changed, 33 insertions(+)

Comments

Heinrich Schuchardt July 14, 2023, 1:39 p.m. UTC | #1 
On 14.07.23 07:44, Masahisa Kojima wrote:
> This is a preparation to add the EFI_RAM_DISK_PROTOCOL.
> This commit adds the RAM disk device path structure
> and text conversion to Device Path to Text Protocol.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> No update since v1
>
>   include/efi_api.h                        | 19 +++++++++++++++++++
>   lib/efi_loader/efi_device_path_to_text.c | 14 ++++++++++++++
>   2 files changed, 33 insertions(+)
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 55a4c989fc..4ee4a1b5e9 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -682,6 +682,7 @@ struct efi_device_path_uri {
>   #  define DEVICE_PATH_SUB_TYPE_CDROM_PATH	0x02
>   #  define DEVICE_PATH_SUB_TYPE_VENDOR_PATH	0x03
>   #  define DEVICE_PATH_SUB_TYPE_FILE_PATH	0x04
> +#  define DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH	0x09
>
>   struct efi_device_path_hard_drive_path {
>   	struct efi_device_path dp;
> @@ -705,6 +706,24 @@ struct efi_device_path_file_path {
>   	u16 str[];
>   } __packed;
>
> +/* This GUID defines a RAM Disk supporting a raw disk format in volatile memory */
> +#define EFI_VIRTUAL_DISK_GUID \
> +	EFI_GUID(0x77ab535a, 0x45fc, 0x624b, \
> +	0x55, 0x60, 0xf7, 0xb2, 0x81, 0xd1, 0xf9, 0x6e)
> +
> +/* This GUID defines a RAM Disk supporting an ISO image in volatile memory */
> +#define EFI_VIRTUAL_CD_GUID \
> +	EFI_GUID(0x3d5abd30, 0x4175, 0x87ce, \
> +		 0x6d, 0x64, 0xd2, 0xad, 0xe5, 0x23, 0xc4, 0xbb)
> +
> +struct efi_device_path_ram_disk_path {
> +	struct efi_device_path dp;
> +	u64 starting_address;
> +	u64 ending_address;
> +	efi_guid_t disk_type_guid;
> +	u16 disk_instance;
> +} __packed;
> +
>   #define EFI_BLOCK_IO_PROTOCOL_GUID \
>   	EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \
>   		 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
> index 8c76d8be60..4395e79f33 100644
> --- a/lib/efi_loader/efi_device_path_to_text.c
> +++ b/lib/efi_loader/efi_device_path_to_text.c
> @@ -324,6 +324,20 @@ static char *dp_media(char *s, struct efi_device_path *dp)
>   		free(buffer);
>   		break;
>   	}
> +	case DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH: {
> +		struct efi_device_path_ram_disk_path *rddp =
> +			(struct efi_device_path_ram_disk_path *)dp;
> +		u64 start;
> +		u64 end;
> +
> +		/* Copy from packed structure to aligned memory */
> +		memcpy(&start, &rddp->starting_address, sizeof(start));
> +		memcpy(&end, &rddp->ending_address, sizeof(end));
> +
> +		s += sprintf(s, "RamDisk(0x%llx,%llx,%pUl,0x%x)", start, end,
> +			     &rddp->disk_type_guid, rddp->disk_instance);

If there is no alignment guarantee for starting_address, then the same
is true for disk_instance which may spread over two u64 blocks.

In case of DEVICE_PATH_SUB_TYPE_MEMORY we don't use memcpy() to align u64.

I don't think we call device_path_to_text before allow_unaligned().

There is a family of functions like get_unaligned_le64() if we should
ever need to a align a value. Or we could copy the whole device path node.

Best regards

Heinrich

> +		break;
> +	}
>   	default:
>   		s = dp_unknown(s, dp);
>   		break;
Masahisa Kojima July 18, 2023, 1:31 a.m. UTC | #2 
Hi Heinrich,

On Fri, 14 Jul 2023 at 22:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 14.07.23 07:44, Masahisa Kojima wrote:
> > This is a preparation to add the EFI_RAM_DISK_PROTOCOL.
> > This commit adds the RAM disk device path structure
> > and text conversion to Device Path to Text Protocol.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > No update since v1
> >
> >   include/efi_api.h                        | 19 +++++++++++++++++++
> >   lib/efi_loader/efi_device_path_to_text.c | 14 ++++++++++++++
> >   2 files changed, 33 insertions(+)
> >
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index 55a4c989fc..4ee4a1b5e9 100644
> > --- a/include/efi_api.h
> > +++ b/include/efi_api.h
> > @@ -682,6 +682,7 @@ struct efi_device_path_uri {
> >   #  define DEVICE_PATH_SUB_TYPE_CDROM_PATH   0x02
> >   #  define DEVICE_PATH_SUB_TYPE_VENDOR_PATH  0x03
> >   #  define DEVICE_PATH_SUB_TYPE_FILE_PATH    0x04
> > +#  define DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH 0x09
> >
> >   struct efi_device_path_hard_drive_path {
> >       struct efi_device_path dp;
> > @@ -705,6 +706,24 @@ struct efi_device_path_file_path {
> >       u16 str[];
> >   } __packed;
> >
> > +/* This GUID defines a RAM Disk supporting a raw disk format in volatile memory */
> > +#define EFI_VIRTUAL_DISK_GUID \
> > +     EFI_GUID(0x77ab535a, 0x45fc, 0x624b, \
> > +     0x55, 0x60, 0xf7, 0xb2, 0x81, 0xd1, 0xf9, 0x6e)
> > +
> > +/* This GUID defines a RAM Disk supporting an ISO image in volatile memory */
> > +#define EFI_VIRTUAL_CD_GUID \
> > +     EFI_GUID(0x3d5abd30, 0x4175, 0x87ce, \
> > +              0x6d, 0x64, 0xd2, 0xad, 0xe5, 0x23, 0xc4, 0xbb)
> > +
> > +struct efi_device_path_ram_disk_path {
> > +     struct efi_device_path dp;
> > +     u64 starting_address;
> > +     u64 ending_address;
> > +     efi_guid_t disk_type_guid;
> > +     u16 disk_instance;
> > +} __packed;
> > +
> >   #define EFI_BLOCK_IO_PROTOCOL_GUID \
> >       EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \
> >                0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> > diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
> > index 8c76d8be60..4395e79f33 100644
> > --- a/lib/efi_loader/efi_device_path_to_text.c
> > +++ b/lib/efi_loader/efi_device_path_to_text.c
> > @@ -324,6 +324,20 @@ static char *dp_media(char *s, struct efi_device_path *dp)
> >               free(buffer);
> >               break;
> >       }
> > +     case DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH: {
> > +             struct efi_device_path_ram_disk_path *rddp =
> > +                     (struct efi_device_path_ram_disk_path *)dp;
> > +             u64 start;
> > +             u64 end;
> > +
> > +             /* Copy from packed structure to aligned memory */
> > +             memcpy(&start, &rddp->starting_address, sizeof(start));
> > +             memcpy(&end, &rddp->ending_address, sizeof(end));
> > +
> > +             s += sprintf(s, "RamDisk(0x%llx,%llx,%pUl,0x%x)", start, end,
> > +                          &rddp->disk_type_guid, rddp->disk_instance);
>
> If there is no alignment guarantee for starting_address, then the same
> is true for disk_instance which may spread over two u64 blocks.

disk_instance is a u16 field, so it is aligned.

>
> In case of DEVICE_PATH_SUB_TYPE_MEMORY we don't use memcpy() to align u64.
>
> I don't think we call device_path_to_text before allow_unaligned().
>
> There is a family of functions like get_unaligned_le64() if we should
> ever need to a align a value. Or we could copy the whole device path node.

OK, I will use get_unaligned_le64().

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> > +             break;
> > +     }
> >       default:
> >               s = dp_unknown(s, dp);
> >               break;
>
Heinrich Schuchardt July 18, 2023, 5:57 a.m. UTC | #3 
Am 18. Juli 2023 03:31:30 MESZ schrieb Masahisa Kojima <masahisa.kojima@linaro.org>:
>Hi Heinrich,
>
>On Fri, 14 Jul 2023 at 22:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 14.07.23 07:44, Masahisa Kojima wrote:
>> > This is a preparation to add the EFI_RAM_DISK_PROTOCOL.
>> > This commit adds the RAM disk device path structure
>> > and text conversion to Device Path to Text Protocol.
>> >
>> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
>> > ---
>> > No update since v1
>> >
>> >   include/efi_api.h                        | 19 +++++++++++++++++++
>> >   lib/efi_loader/efi_device_path_to_text.c | 14 ++++++++++++++
>> >   2 files changed, 33 insertions(+)
>> >
>> > diff --git a/include/efi_api.h b/include/efi_api.h
>> > index 55a4c989fc..4ee4a1b5e9 100644
>> > --- a/include/efi_api.h
>> > +++ b/include/efi_api.h
>> > @@ -682,6 +682,7 @@ struct efi_device_path_uri {
>> >   #  define DEVICE_PATH_SUB_TYPE_CDROM_PATH   0x02
>> >   #  define DEVICE_PATH_SUB_TYPE_VENDOR_PATH  0x03
>> >   #  define DEVICE_PATH_SUB_TYPE_FILE_PATH    0x04
>> > +#  define DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH 0x09
>> >
>> >   struct efi_device_path_hard_drive_path {
>> >       struct efi_device_path dp;
>> > @@ -705,6 +706,24 @@ struct efi_device_path_file_path {
>> >       u16 str[];
>> >   } __packed;
>> >
>> > +/* This GUID defines a RAM Disk supporting a raw disk format in volatile memory */
>> > +#define EFI_VIRTUAL_DISK_GUID \
>> > +     EFI_GUID(0x77ab535a, 0x45fc, 0x624b, \
>> > +     0x55, 0x60, 0xf7, 0xb2, 0x81, 0xd1, 0xf9, 0x6e)
>> > +
>> > +/* This GUID defines a RAM Disk supporting an ISO image in volatile memory */
>> > +#define EFI_VIRTUAL_CD_GUID \
>> > +     EFI_GUID(0x3d5abd30, 0x4175, 0x87ce, \
>> > +              0x6d, 0x64, 0xd2, 0xad, 0xe5, 0x23, 0xc4, 0xbb)
>> > +
>> > +struct efi_device_path_ram_disk_path {
>> > +     struct efi_device_path dp;
>> > +     u64 starting_address;
>> > +     u64 ending_address;
>> > +     efi_guid_t disk_type_guid;
>> > +     u16 disk_instance;
>> > +} __packed;
>> > +
>> >   #define EFI_BLOCK_IO_PROTOCOL_GUID \
>> >       EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \
>> >                0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>> > diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
>> > index 8c76d8be60..4395e79f33 100644
>> > --- a/lib/efi_loader/efi_device_path_to_text.c
>> > +++ b/lib/efi_loader/efi_device_path_to_text.c
>> > @@ -324,6 +324,20 @@ static char *dp_media(char *s, struct efi_device_path *dp)
>> >               free(buffer);
>> >               break;
>> >       }
>> > +     case DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH: {
>> > +             struct efi_device_path_ram_disk_path *rddp =
>> > +                     (struct efi_device_path_ram_disk_path *)dp;
>> > +             u64 start;
>> > +             u64 end;
>> > +
>> > +             /* Copy from packed structure to aligned memory */
>> > +             memcpy(&start, &rddp->starting_address, sizeof(start));
>> > +             memcpy(&end, &rddp->ending_address, sizeof(end));
>> > +
>> > +             s += sprintf(s, "RamDisk(0x%llx,%llx,%pUl,0x%x)", start, end,
>> > +                          &rddp->disk_type_guid, rddp->disk_instance);
>>
>> If there is no alignment guarantee for starting_address, then the same
>> is true for disk_instance which may spread over two u64 blocks.
>
>disk_instance is a u16 field, so it is aligned.

A preceding node, e.g. VenHw(), may have an uneven length?

>
>>
>> In case of DEVICE_PATH_SUB_TYPE_MEMORY we don't use memcpy() to align u64.
>>
>> I don't think we call device_path_to_text before allow_unaligned().
>>
>> There is a family of functions like get_unaligned_le64() if we should
>> ever need to a align a value. Or we could copy the whole device path node.
>
>OK, I will use get_unaligned_le64().

Why do you think this is necessary?

Best regards

Heinrich 

>
>Thanks,
>Masahisa Kojima
>
>>
>> Best regards
>>
>> Heinrich
>>
>> > +             break;
>> > +     }
>> >       default:
>> >               s = dp_unknown(s, dp);
>> >               break;
>>
Masahisa Kojima July 18, 2023, 6:40 a.m. UTC | #4 
On Tue, 18 Jul 2023 at 14:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> Am 18. Juli 2023 03:31:30 MESZ schrieb Masahisa Kojima <masahisa.kojima@linaro.org>:
> >Hi Heinrich,
> >
> >On Fri, 14 Jul 2023 at 22:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> On 14.07.23 07:44, Masahisa Kojima wrote:
> >> > This is a preparation to add the EFI_RAM_DISK_PROTOCOL.
> >> > This commit adds the RAM disk device path structure
> >> > and text conversion to Device Path to Text Protocol.
> >> >
> >> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> >> > ---
> >> > No update since v1
> >> >
> >> >   include/efi_api.h                        | 19 +++++++++++++++++++
> >> >   lib/efi_loader/efi_device_path_to_text.c | 14 ++++++++++++++
> >> >   2 files changed, 33 insertions(+)
> >> >
> >> > diff --git a/include/efi_api.h b/include/efi_api.h
> >> > index 55a4c989fc..4ee4a1b5e9 100644
> >> > --- a/include/efi_api.h
> >> > +++ b/include/efi_api.h
> >> > @@ -682,6 +682,7 @@ struct efi_device_path_uri {
> >> >   #  define DEVICE_PATH_SUB_TYPE_CDROM_PATH   0x02
> >> >   #  define DEVICE_PATH_SUB_TYPE_VENDOR_PATH  0x03
> >> >   #  define DEVICE_PATH_SUB_TYPE_FILE_PATH    0x04
> >> > +#  define DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH 0x09
> >> >
> >> >   struct efi_device_path_hard_drive_path {
> >> >       struct efi_device_path dp;
> >> > @@ -705,6 +706,24 @@ struct efi_device_path_file_path {
> >> >       u16 str[];
> >> >   } __packed;
> >> >
> >> > +/* This GUID defines a RAM Disk supporting a raw disk format in volatile memory */
> >> > +#define EFI_VIRTUAL_DISK_GUID \
> >> > +     EFI_GUID(0x77ab535a, 0x45fc, 0x624b, \
> >> > +     0x55, 0x60, 0xf7, 0xb2, 0x81, 0xd1, 0xf9, 0x6e)
> >> > +
> >> > +/* This GUID defines a RAM Disk supporting an ISO image in volatile memory */
> >> > +#define EFI_VIRTUAL_CD_GUID \
> >> > +     EFI_GUID(0x3d5abd30, 0x4175, 0x87ce, \
> >> > +              0x6d, 0x64, 0xd2, 0xad, 0xe5, 0x23, 0xc4, 0xbb)
> >> > +
> >> > +struct efi_device_path_ram_disk_path {
> >> > +     struct efi_device_path dp;
> >> > +     u64 starting_address;
> >> > +     u64 ending_address;
> >> > +     efi_guid_t disk_type_guid;
> >> > +     u16 disk_instance;
> >> > +} __packed;
> >> > +
> >> >   #define EFI_BLOCK_IO_PROTOCOL_GUID \
> >> >       EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \
> >> >                0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> >> > diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
> >> > index 8c76d8be60..4395e79f33 100644
> >> > --- a/lib/efi_loader/efi_device_path_to_text.c
> >> > +++ b/lib/efi_loader/efi_device_path_to_text.c
> >> > @@ -324,6 +324,20 @@ static char *dp_media(char *s, struct efi_device_path *dp)
> >> >               free(buffer);
> >> >               break;
> >> >       }
> >> > +     case DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH: {
> >> > +             struct efi_device_path_ram_disk_path *rddp =
> >> > +                     (struct efi_device_path_ram_disk_path *)dp;
> >> > +             u64 start;
> >> > +             u64 end;
> >> > +
> >> > +             /* Copy from packed structure to aligned memory */
> >> > +             memcpy(&start, &rddp->starting_address, sizeof(start));
> >> > +             memcpy(&end, &rddp->ending_address, sizeof(end));
> >> > +
> >> > +             s += sprintf(s, "RamDisk(0x%llx,%llx,%pUl,0x%x)", start, end,
> >> > +                          &rddp->disk_type_guid, rddp->disk_instance);
> >>
> >> If there is no alignment guarantee for starting_address, then the same
> >> is true for disk_instance which may spread over two u64 blocks.
> >
> >disk_instance is a u16 field, so it is aligned.
>
> A preceding node, e.g. VenHw(), may have an uneven length?
>
> >
> >>
> >> In case of DEVICE_PATH_SUB_TYPE_MEMORY we don't use memcpy() to align u64.
> >>
> >> I don't think we call device_path_to_text before allow_unaligned().
> >>
> >> There is a family of functions like get_unaligned_le64() if we should
> >> ever need to a align a value. Or we could copy the whole device path node.
> >
> >OK, I will use get_unaligned_le64().
>
> Why do you think this is necessary?

Sorry, I misunderstood your comment.
allow_unaligned() is called in efi_init_early(), device_path_to_text
will not be called
before allow_unaligned(). So we don't need to care about alignment in
device_path_to_text
like DEVICE_PATH_SUB_TYPE_MEMORY is doing.

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> >
> >Thanks,
> >Masahisa Kojima
> >
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >> > +             break;
> >> > +     }
> >> >       default:
> >> >               s = dp_unknown(s, dp);
> >> >               break;
> >>
diff mbox series

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index 55a4c989fc..4ee4a1b5e9 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -682,6 +682,7 @@  struct efi_device_path_uri {
 #  define DEVICE_PATH_SUB_TYPE_CDROM_PATH	0x02
 #  define DEVICE_PATH_SUB_TYPE_VENDOR_PATH	0x03
 #  define DEVICE_PATH_SUB_TYPE_FILE_PATH	0x04
+#  define DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH	0x09
 
 struct efi_device_path_hard_drive_path {
 	struct efi_device_path dp;
@@ -705,6 +706,24 @@  struct efi_device_path_file_path {
 	u16 str[];
 } __packed;
 
+/* This GUID defines a RAM Disk supporting a raw disk format in volatile memory */
+#define EFI_VIRTUAL_DISK_GUID \
+	EFI_GUID(0x77ab535a, 0x45fc, 0x624b, \
+	0x55, 0x60, 0xf7, 0xb2, 0x81, 0xd1, 0xf9, 0x6e)
+
+/* This GUID defines a RAM Disk supporting an ISO image in volatile memory */
+#define EFI_VIRTUAL_CD_GUID \
+	EFI_GUID(0x3d5abd30, 0x4175, 0x87ce, \
+		 0x6d, 0x64, 0xd2, 0xad, 0xe5, 0x23, 0xc4, 0xbb)
+
+struct efi_device_path_ram_disk_path {
+	struct efi_device_path dp;
+	u64 starting_address;
+	u64 ending_address;
+	efi_guid_t disk_type_guid;
+	u16 disk_instance;
+} __packed;
+
 #define EFI_BLOCK_IO_PROTOCOL_GUID \
 	EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \
 		 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
index 8c76d8be60..4395e79f33 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -324,6 +324,20 @@  static char *dp_media(char *s, struct efi_device_path *dp)
 		free(buffer);
 		break;
 	}
+	case DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH: {
+		struct efi_device_path_ram_disk_path *rddp =
+			(struct efi_device_path_ram_disk_path *)dp;
+		u64 start;
+		u64 end;
+
+		/* Copy from packed structure to aligned memory */
+		memcpy(&start, &rddp->starting_address, sizeof(start));
+		memcpy(&end, &rddp->ending_address, sizeof(end));
+
+		s += sprintf(s, "RamDisk(0x%llx,%llx,%pUl,0x%x)", start, end,
+			     &rddp->disk_type_guid, rddp->disk_instance);
+		break;
+	}
 	default:
 		s = dp_unknown(s, dp);
 		break;