[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 1/1] hw/core: add qom getter for kernel-irqc
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [RFC PATCH 1/1] hw/core: add qom getter for kernel-irqchip property |
Date: |
Thu, 27 Dec 2018 18:33:12 -0200 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
On Tue, Dec 25, 2018 at 01:20:05PM +0800, Peter Xu wrote:
> On Mon, Dec 24, 2018 at 06:52:35AM -0500, Wainer dos Santos Moschetta wrote:
> > Allows to access the kernel-irqchip property of a Machine
> > Class object via QOM get method.
> >
> > Before this patch the property cannot be read although it is
> > listed by qom-list:
> >
> > qemu-system-x86_64 -M q35,accel=kvm,kernel-irqchip=split (...)
> > -> {"execute": "qom-list", "arguments": {"path": "/machine"}}
> > <- {"return": [{"name": "type", "type": "string"}, (...) , {"name":
> > "kernel-irqchip", "type": "on|off|split"} (...)}
> > -> {"execute": "qom-get", "arguments": {"path": "/machine", "property":
> > "kernel-irqchip"}}
> > <- {"error": {"class": "GenericError", "desc": "Insufficient permission to
> > perform this operation"}}
> >
> > It is implemented the machine_get_kernel_irqchip() method,
> > which evaluates the kernel_irqchip_allowed, *_required, and
> > *_split fields to determine the value of kernel-irqchip. Note: we
> > assume there is not inconsistency on the value of those fields.
> >
> > Then with this change in place, it works as expected:
> >
> > qemu-system-x86_64 -M q35,accel=kvm,kernel-irqchip=split (...)
> > -> {"execute": "qom-get", "arguments": {"path": "/machine", "property":
> > "kernel-irqchip"}}
> > <- {"return": "split"}
> >
> > Signed-off-by: Wainer dos Santos Moschetta <address@hidden>
> > ---
> > hw/core/machine.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index c51423b647..f61003f8f2 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -37,6 +37,21 @@ static void machine_set_accel(Object *obj, const char
> > *value, Error **errp)
> > ms->accel = g_strdup(value);
> > }
> >
> > +static void machine_get_kernel_irqchip(Object *obj, Visitor *v,
> > + const char *name, void *opaque,
> > + Error **errp)
> > +{
> > + MachineState *ms = MACHINE(obj);
> > + OnOffSplit mode;
> > +
> > + if (ms->kernel_irqchip_split) {
> > + mode = ON_OFF_SPLIT_SPLIT;
> > + } else {
> > + mode = (ms->kernel_irqchip_allowed && ms->kernel_irqchip_required)
> > ?
> > + ON_OFF_SPLIT_ON : ON_OFF_SPLIT_OFF;
>
> Hi, Wainer,
>
> The new interface seems to be a good thing, though the implementation
> might be incorrect here. AFAIU these parameters only decide "how we
> want to choose the kernel-irqchip parameter" rather than the real
> result, which could depend on more (e.g., whether KVM is used).
>
> IMHO you should simply fetch the results from kvm_irqchip_in_kernel()
> and kvm_irqchip_is_split() where the real results are stored.
Agreed. Knowing the actual values of kvm_irqchip_*() would be
even more useful for test code (this way we would be testing all
the KVM irqchip initialization code, not just a few lines of
property getter/seter code).
--
Eduardo