On 05/07/2022 10:39, Daniel Henrique Barboza wrote:
On 7/5/22 03:51, Mark Cave-Ayland wrote:
On 04/07/2022 18:34, Cédric Le Goater wrote:
On 7/2/22 15:34, Daniel Henrique Barboza wrote:
On 7/2/22 03:24, Cédric Le Goater wrote:
On 6/30/22 21:42, Daniel Henrique Barboza wrote:
The function can't just return 0 whether an error happened and call it a
day. We must provide a way of letting callers know if the zero return is
legitimate or due to an error.
Add an Error pointer to kvmppc_read_int_cpu_dt() that will be filled
with an appropriate error, if one occurs. Callers are then free to pass
an Error pointer and handle it.
Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>
---
target/ppc/kvm.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 109823136d..bc17437097 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1925,15 +1925,17 @@ static uint64_t kvmppc_read_int_dt(const char *filename)
/*
* Read a CPU node property from the host device tree that's a single
- * integer (32-bit or 64-bit). Returns 0 if anything goes wrong
- * (can't find or open the property, or doesn't understand the format)
+ * integer (32-bit or 64-bit). Returns 0 and set errp if anything goes
+ * wrong (can't find or open the property, or doesn't understand the
+ * format)
*/
-static uint64_t kvmppc_read_int_cpu_dt(const char *propname)
+static uint64_t kvmppc_read_int_cpu_dt(const char *propname, Error **errp)
{
char buf[PATH_MAX], *tmp;
uint64_t val;
if (kvmppc_find_cpu_dt(buf, sizeof(buf))) {
+ error_setg(errp, "Failed to read CPU property %s", propname);
return 0;
}
@@ -1946,12 +1948,12 @@ static uint64_t kvmppc_read_int_cpu_dt(const char
*propname)
uint64_t kvmppc_get_clockfreq(void)
{
- return kvmppc_read_int_cpu_dt("clock-frequency");
+ return kvmppc_read_int_cpu_dt("clock-frequency", NULL);
This should be fatal. no ?
I'm not sure. I went under the assumption that there might be some weird
condition where 'clock-frequency' might be missing from the DT, and this
is why we're not exiting out immediately.
That said, the advantage of turning this into fatal is that we won't need
all the other patches that handles error on this function. We're going to
assume that if 'clock-frequency' is missing then we can't boot. I'm okay
with that.
I think so. Some machines behave badly when 'clock-frequency' is bogus,
division by zero, no console, etc. We could check easily with pseries
which is the only KVM PPC platform.
Well not quite true ;) I haven't tested it during the last release cycle, but the
Mac machines were still working fine to boot OS X with KVM-PR on my G4 Mac Mini
last time I checked.
We can't just error_fatal by default then.
Each machine should be able to determine how to handle this missing DT
element. If I want to error_fatal for pseries then I can do so in patch
9/9, but other than that I'll keep the existing behavior.
This wouldn't be a problem for the Mac machines since they pass the clock frequency
from QEMU to OpenBIOS using the fw_cfg interface which builds the tree itself rather
than using FDT. So I think using error_fatal should still be fine.
ATB,
Mark.
