diff mbox

[v5,1/6] boot: extend get_boot_devices_list() to ignore suffixes

Message ID 1392904246-15575-2-git-send-email-aik@ozlabs.ru
State New
Headers show

Commit Message

Alexey Kardashevskiy Feb. 20, 2014, 1:50 p.m. UTC
As suffixes do not make sense for sPAPR's device tree and
there is no way to filter them out on the BusState::get_fw_dev_path
level, let's add an ability for the external caller to specify
whether to apply suffixes or not.

We could handle suffixes in SLOF (ignore for now) but this would require
serious rework in the node opening code in SLOF which has no obvious
benefit for the currently emulated sPAPR machine.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* changed commit message about not having plans in SLOF rework as it is scary :)
---
 hw/nvram/fw_cfg.c       | 2 +-
 include/sysemu/sysemu.h | 2 +-
 vl.c                    | 8 +++++---
 3 files changed, 7 insertions(+), 5 deletions(-)

Comments

Paolo Bonzini Feb. 20, 2014, 1:55 p.m. UTC | #1
Il 20/02/2014 14:50, Alexey Kardashevskiy ha scritto:
> As suffixes do not make sense for sPAPR's device tree and
> there is no way to filter them out on the BusState::get_fw_dev_path
> level, let's add an ability for the external caller to specify
> whether to apply suffixes or not.
>
> We could handle suffixes in SLOF (ignore for now) but this would require
> serious rework in the node opening code in SLOF which has no obvious
> benefit for the currently emulated sPAPR machine.

For the record, the commit message is not entirely correct in presenting 
the situation.  QEMU *does not care in any way* of benefits for the 
currently emulated sPAPR machine.  The benefit would be to QEMU in 
having simpler code.

You just got a wildcard because Forth is scary. :)

Paolo

> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v4:
> * changed commit message about not having plans in SLOF rework as it is scary :)
Alexey Kardashevskiy Feb. 20, 2014, 2:03 p.m. UTC | #2
On 02/21/2014 12:55 AM, Paolo Bonzini wrote:
> Il 20/02/2014 14:50, Alexey Kardashevskiy ha scritto:
>> As suffixes do not make sense for sPAPR's device tree and
>> there is no way to filter them out on the BusState::get_fw_dev_path
>> level, let's add an ability for the external caller to specify
>> whether to apply suffixes or not.
>>
>> We could handle suffixes in SLOF (ignore for now) but this would require
>> serious rework in the node opening code in SLOF which has no obvious
>> benefit for the currently emulated sPAPR machine.
> 
> For the record, the commit message is not entirely correct in presenting
> the situation.  QEMU *does not care in any way* of benefits for the
> currently emulated sPAPR machine.  The benefit would be to QEMU in having
> simpler code.
> 
> You just got a wildcard because Forth is scary. :)

I know :) Should I remove that part and replace it with the "scary" one?


> Paolo
> 
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v4:
>> * changed commit message about not having plans in SLOF rework as it is
>> scary :)
> 
>
Paolo Bonzini Feb. 20, 2014, 2:05 p.m. UTC | #3
Il 20/02/2014 15:03, Alexey Kardashevskiy ha scritto:
> On 02/21/2014 12:55 AM, Paolo Bonzini wrote:
>> Il 20/02/2014 14:50, Alexey Kardashevskiy ha scritto:
>>> As suffixes do not make sense for sPAPR's device tree and
>>> there is no way to filter them out on the BusState::get_fw_dev_path
>>> level, let's add an ability for the external caller to specify
>>> whether to apply suffixes or not.
>>>
>>> We could handle suffixes in SLOF (ignore for now) but this would require
>>> serious rework in the node opening code in SLOF which has no obvious
>>> benefit for the currently emulated sPAPR machine.
>>
>> For the record, the commit message is not entirely correct in presenting
>> the situation.  QEMU *does not care in any way* of benefits for the
>> currently emulated sPAPR machine.  The benefit would be to QEMU in having
>> simpler code.
>>
>> You just got a wildcard because Forth is scary. :)
>
> I know :) Should I remove that part and replace it with the "scary" one?

No, unless you have to respin for other reasons (I hope not).

Paolo
Alexey Kardashevskiy March 13, 2014, 3:32 a.m. UTC | #4
On 02/21/2014 01:05 AM, Paolo Bonzini wrote:
> Il 20/02/2014 15:03, Alexey Kardashevskiy ha scritto:
>> On 02/21/2014 12:55 AM, Paolo Bonzini wrote:
>>> Il 20/02/2014 14:50, Alexey Kardashevskiy ha scritto:
>>>> As suffixes do not make sense for sPAPR's device tree and
>>>> there is no way to filter them out on the BusState::get_fw_dev_path
>>>> level, let's add an ability for the external caller to specify
>>>> whether to apply suffixes or not.
>>>>
>>>> We could handle suffixes in SLOF (ignore for now) but this would require
>>>> serious rework in the node opening code in SLOF which has no obvious
>>>> benefit for the currently emulated sPAPR machine.
>>>
>>> For the record, the commit message is not entirely correct in presenting
>>> the situation.  QEMU *does not care in any way* of benefits for the
>>> currently emulated sPAPR machine.  The benefit would be to QEMU in having
>>> simpler code.
>>>
>>> You just got a wildcard because Forth is scary. :)
>>
>> I know :) Should I remove that part and replace it with the "scary" one?
> 
> No, unless you have to respin for other reasons (I hope not).

I am about to respin the series against the latest QOM stuff and I am about
to change commit message in the "The benefit would be to QEMU in having
simpler code" part - what part of QEMU gets simpler? I am avoiding fixing
SLOF and I am fixing QEMU so QEMU gets (liiiiittle) bit more complicated
but not simpler :) What do I miss?
Paolo Bonzini March 13, 2014, 8 a.m. UTC | #5
Il 13/03/2014 04:32, Alexey Kardashevskiy ha scritto:
> I am about to respin the series against the latest QOM stuff and I am about
> to change commit message in the "The benefit would be to QEMU in having
> simpler code" part - what part of QEMU gets simpler? I am avoiding fixing
> SLOF and I am fixing QEMU so QEMU gets (liiiiittle) bit more complicated
> but not simpler

The benefit would be to QEMU in having consistent naming of devices 
across all architectures.

Or just ignore me.

Paolo
diff mbox

Patch

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index ee96c16..42fa448 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -504,7 +504,7 @@  static void fw_cfg_machine_ready(struct Notifier *n, void *data)
 {
     size_t len;
     FWCfgState *s = container_of(n, FWCfgState, machine_ready);
-    char *bootindex = get_boot_devices_list(&len);
+    char *bootindex = get_boot_devices_list(&len, false);
 
     fw_cfg_add_file(s, "bootorder", (uint8_t*)bootindex, len);
 }
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 495dae8..2b71a4a 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -185,7 +185,7 @@  void rtc_change_mon_event(struct tm *tm);
 
 void add_boot_device_path(int32_t bootindex, DeviceState *dev,
                           const char *suffix);
-char *get_boot_devices_list(size_t *size);
+char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
 
 DeviceState *get_boot_device(uint32_t position);
 
diff --git a/vl.c b/vl.c
index 57e98fa..01ab7e4 100644
--- a/vl.c
+++ b/vl.c
@@ -1166,7 +1166,7 @@  DeviceState *get_boot_device(uint32_t position)
  * memory pointed by "size" is assigned total length of the array in bytes
  *
  */
-char *get_boot_devices_list(size_t *size)
+char *get_boot_devices_list(size_t *size, bool ignore_suffixes)
 {
     FWBootEntry *i;
     size_t total = 0;
@@ -1181,7 +1181,7 @@  char *get_boot_devices_list(size_t *size)
             assert(devpath);
         }
 
-        if (i->suffix && devpath) {
+        if (i->suffix && !ignore_suffixes && devpath) {
             size_t bootpathlen = strlen(devpath) + strlen(i->suffix) + 1;
 
             bootpath = g_malloc(bootpathlen);
@@ -1189,9 +1189,11 @@  char *get_boot_devices_list(size_t *size)
             g_free(devpath);
         } else if (devpath) {
             bootpath = devpath;
-        } else {
+        } else if (!ignore_suffixes) {
             assert(i->suffix);
             bootpath = g_strdup(i->suffix);
+        } else {
+            bootpath = g_strdup("");
         }
 
         if (total) {