qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [qemu-s390x] [PATCH] s390-bios: Skip bootmap signature


From: Christian Borntraeger
Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH] s390-bios: Skip bootmap signature entries
Date: Mon, 6 May 2019 12:45:36 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1


On 06.05.19 12:30, Cornelia Huck wrote:
> On Mon, 6 May 2019 12:14:10 +0200
> Christian Borntraeger <address@hidden> wrote:
> 
>> On 06.05.19 12:01, David Hildenbrand wrote:
>>> On 29.04.19 15:09, Jason J. Herne wrote:  
>>>> Newer versions of zipl have the ability to write signature entries to the 
>>>> boot
>>>> script for secure boot. We don't yet support secure boot, but we need to 
>>>> skip
>>>> over signature entries while reading the boot script in order to maintain 
>>>> our
>>>> ability to boot guest operating systems that have a secure bootloader.
>>>>
>>>> Signed-off-by: Jason J. Herne <address@hidden>
>>>> Reviewed-by: Farhan Ali <address@hidden>
>>>> ---
>>>>  pc-bios/s390-ccw/bootmap.c | 19 +++++++++++++++++--
>>>>  pc-bios/s390-ccw/bootmap.h | 10 ++++++----
>>>>  2 files changed, 23 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
>>>> index 7aef65a..d13b7cb 100644
>>>> --- a/pc-bios/s390-ccw/bootmap.c
>>>> +++ b/pc-bios/s390-ccw/bootmap.c
>>>> @@ -254,7 +254,14 @@ static void run_eckd_boot_script(block_number_t 
>>>> bmt_block_nr,
>>>>      memset(sec, FREE_SPACE_FILLER, sizeof(sec));
>>>>      read_block(block_nr, sec, "Cannot read Boot Map Script");
>>>>  
>>>> -    for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD; i++) {
>>>> +    for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD ||
>>>> +                bms->entry[i].type == BOOT_SCRIPT_SIGNATURE; i++) {
>>>> +
>>>> +        /* We don't support secure boot yet, so we skip signature entries 
>>>> */
>>>> +        if (bms->entry[i].type == BOOT_SCRIPT_SIGNATURE) {
>>>> +            continue;
>>>> +        }
>>>> +
>>>>          address = bms->entry[i].address.load_address;
>>>>          block_nr = eckd_block_num(&bms->entry[i].blkptr.xeckd.bptr.chs);
>>>>  
>>>> @@ -489,7 +496,15 @@ static void zipl_run(ScsiBlockPtr *pte)
>>>>  
>>>>      /* Load image(s) into RAM */
>>>>      entry = (ComponentEntry *)(&header[1]);
>>>> -    while (entry->component_type == ZIPL_COMP_ENTRY_LOAD) {
>>>> +    while (entry->component_type == ZIPL_COMP_ENTRY_LOAD ||
>>>> +           entry->component_type == ZIPL_COMP_ENTRY_SIGNATURE) {
>>>> +
>>>> +        /* We don't support secure boot yet, so we skip signature entries 
>>>> */
>>>> +        if (entry->component_type == ZIPL_COMP_ENTRY_SIGNATURE) {
>>>> +            entry++;
>>>> +            continue;
>>>> +        }
>>>> +
>>>>          zipl_load_segment(entry);
>>>>  
>>>>          entry++;
>>>> diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
>>>> index a085212..94f53a5 100644
>>>> --- a/pc-bios/s390-ccw/bootmap.h
>>>> +++ b/pc-bios/s390-ccw/bootmap.h
>>>> @@ -98,8 +98,9 @@ typedef struct ScsiMbr {
>>>>  #define ZIPL_COMP_HEADER_IPL    0x00
>>>>  #define ZIPL_COMP_HEADER_DUMP   0x01
>>>>  
>>>> -#define ZIPL_COMP_ENTRY_LOAD    0x02
>>>> -#define ZIPL_COMP_ENTRY_EXEC    0x01
>>>> +#define ZIPL_COMP_ENTRY_EXEC      0x01
>>>> +#define ZIPL_COMP_ENTRY_LOAD      0x02
>>>> +#define ZIPL_COMP_ENTRY_SIGNATURE 0x03
>>>>  
>>>>  typedef struct XEckdMbr {
>>>>      uint8_t magic[4];   /* == "xIPL"        */
>>>> @@ -117,8 +118,9 @@ typedef struct BootMapScriptEntry {
>>>>      BootMapPointer blkptr;
>>>>      uint8_t pad[7];
>>>>      uint8_t type;   /* == BOOT_SCRIPT_* */
>>>> -#define BOOT_SCRIPT_EXEC 0x01
>>>> -#define BOOT_SCRIPT_LOAD 0x02
>>>> +#define BOOT_SCRIPT_EXEC      0x01
>>>> +#define BOOT_SCRIPT_LOAD      0x02
>>>> +#define BOOT_SCRIPT_SIGNATURE 0x03
>>>>      union {
>>>>          uint64_t load_address;
>>>>          uint64_t load_psw;
>>>>  
>>>
>>> Naive question from me:
>>>
>>> Can't we place the signatures somewhere else, and instead associate them
>>> with entries? This avoids breaking backwards compatibility for the sake
>>> of signatures we want unmodified zipl loaders to ignore.
>>>   
>>
>> This way is according to hardware(or firmware) architecture for 
>> list-directed IPL, so we have to live with it. In the end zipl can still
>> write the old variant (without secure entries). The default (auto)
>> will detect if the hardware supports secure IPL or not. (via 
>> /sys/firmware/ipl/has_secure)
>> So this toleration support here is necessary for things like installing in an
>> LPAR that has secure boot and then IPLing that disk under a KVM that has not.
> 
> I'm a bit confused here. We want to tolerate booting from a boot record
> that was written on an LPAR that supports secure IPL, which is
> generally reasonable. But: Why is the boot record then written with or
> without signature entries depending on the presence of the feature? How
> 'optional' are those entries, given they may be ignored for portability
> reasons?

zipl does not write those blocks on systems that do will not be able to handle
those. This is something that I asked for. This will make the common use case
still working fine with all KVM systems. (Since KVM parses the zipl layout).

We now still have cases where there are these signature entries. Ignoring
those is also fine, since these records are only relevant if the LPAR is 
enable for secure boot. It is somewhat like on a windows PC. The windows is 
installed with everything necessary to do the secure boot. If you disable
secure boot in the bios, the signatures are just ignored.  (minus Windows 8.1
which showed a watermark on the desktop. Microsoft provided a patch to
disable that watermark).

Makes sense?




reply via email to

[Prev in Thread] Current Thread [Next in Thread]