diff mbox

[RFC,v2,3/7] vl: create power chip device

Message ID 1365136091-26148-4-git-send-email-lig.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

liguang April 5, 2013, 4:28 a.m. UTC
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 vl.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Comments

Paolo Bonzini April 5, 2013, 11:48 a.m. UTC | #1
Il 05/04/2013 06:28, liguang ha scritto:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  vl.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index aeed7f4..a14549e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -171,6 +171,8 @@ int main(int argc, char **argv)
>  #include "ui/qemu-spice.h"
>  #include "qapi/string-input-visitor.h"
>  
> +#include "hw/power.h"
> +
>  //#define DEBUG_NET
>  //#define DEBUG_SLIRP
>  
> @@ -4295,6 +4297,8 @@ int main(int argc, char **argv, char **envp)
>  
>      qdev_machine_init();
>  
> +    qdev_init_nofail(qdev_create(NULL, TYPE_POWER_CHIP));
> +

You cannot just add a random device to the machine.

Perhaps what you want to do is define a QOM interface that some device
in the machine will implement.

But right now this all seems very nebulous.  Honestly, I read the
patches and I have no idea _why_ you are doing this.

Paolo

>      QEMUMachineInitArgs args = { .ram_size = ram_size,
>                                   .boot_device = (boot_devices[0] == '\0') ?
>                                                  machine->boot_order :
>
liguang April 8, 2013, 12:18 a.m. UTC | #2
在 2013-04-05五的 13:48 +0200,Paolo Bonzini写道:
> Il 05/04/2013 06:28, liguang ha scritto:
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > ---
> >  vl.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/vl.c b/vl.c
> > index aeed7f4..a14549e 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -171,6 +171,8 @@ int main(int argc, char **argv)
> >  #include "ui/qemu-spice.h"
> >  #include "qapi/string-input-visitor.h"
> >  
> > +#include "hw/power.h"
> > +
> >  //#define DEBUG_NET
> >  //#define DEBUG_SLIRP
> >  
> > @@ -4295,6 +4297,8 @@ int main(int argc, char **argv, char **envp)
> >  
> >      qdev_machine_init();
> >  
> > +    qdev_init_nofail(qdev_create(NULL, TYPE_POWER_CHIP));
> > +
> 
> You cannot just add a random device to the machine.
> 
> Perhaps what you want to do is define a QOM interface that some device
> in the machine will implement.
> 
> But right now this all seems very nebulous.  Honestly, I read the
> patches and I have no idea _why_ you are doing this.

In short, the purpose is to implement a power control process
just like real world(hardware platform), 
a power chip(controller) connect power signals to other devices,
and devices can do power (on, off, ...) controlled by power
chip(controller).
Is this reasonable?

> >      QEMUMachineInitArgs args = { .ram_size = ram_size,
> >                                   .boot_device = (boot_devices[0] == '\0') ?
> >                                                  machine->boot_order :
> > 
>
Paolo Bonzini April 8, 2013, 9:53 a.m. UTC | #3
Il 08/04/2013 02:18, li guang ha scritto:
> In short, the purpose is to implement a power control process
> just like real world(hardware platform), 
> a power chip(controller) connect power signals to other devices,
> and devices can do power (on, off, ...) controlled by power
> chip(controller).
> Is this reasonable?

It may be reasonable, but it will be board-dependent.  The meaning of
"power on/power off" varies wildly depending on the kind of device, the
platform, etc.

Similarly, "reset" is very different from a power cycle.

Are you trying to implement D-states?

Paolo
liguang April 9, 2013, 7:34 a.m. UTC | #4
在 2013-04-08一的 11:53 +0200,Paolo Bonzini写道:
> Il 08/04/2013 02:18, li guang ha scritto:
> > In short, the purpose is to implement a power control process
> > just like real world(hardware platform), 
> > a power chip(controller) connect power signals to other devices,
> > and devices can do power (on, off, ...) controlled by power
> > chip(controller).
> > Is this reasonable?
> 
> It may be reasonable, but it will be board-dependent.  The meaning of
> "power on/power off" varies wildly depending on the kind of device, the
> platform, etc.
> 
> Similarly, "reset" is very different from a power cycle.
> 
> Are you trying to implement D-states?

Hmm ... maybe, but,
S-states may be more exactly,
S0 -> power on
S3 -> suspend
S5 -> power off
but that are conceptual, 
actually, I just want centralize controlling
of power state transition of whole emulated machine.
Paolo Bonzini April 9, 2013, 7:43 a.m. UTC | #5
Il 09/04/2013 09:34, li guang ha scritto:
> Hmm ... maybe, but,
> S-states may be more exactly,
> S0 -> power on
> S3 -> suspend
> S5 -> power off
> but that are conceptual, 
> actually, I just want centralize controlling
> of power state transition of whole emulated machine.

That is already there:

qemu_system_suspend_request, qemu_register_suspend_notifier
   for S0->S3

qemu_system_wakeup_request, qemu_register_wakeup_notifier
   for S3->S0

qemu_system_powerdown_request, qemu_register_powerdown_notifier
   for Sx->S5

and the reset mechanism for S5->S0.

Most of these should only be used by one "power management device" on
the board (e.g. the PIIX3 or ICH9 power management devices in hw/acpi/*)
and distributed to the other devices via whatever buses exist on the
real machine.

Paolo
liguang April 9, 2013, 8:26 a.m. UTC | #6
在 2013-04-09二的 09:43 +0200,Paolo Bonzini写道:
> Il 09/04/2013 09:34, li guang ha scritto:
> > Hmm ... maybe, but,
> > S-states may be more exactly,
> > S0 -> power on
> > S3 -> suspend
> > S5 -> power off
> > but that are conceptual, 
> > actually, I just want centralize controlling
> > of power state transition of whole emulated machine.
> 
> That is already there:
> 
> qemu_system_suspend_request, qemu_register_suspend_notifier
>    for S0->S3
> 
> qemu_system_wakeup_request, qemu_register_wakeup_notifier
>    for S3->S0
> 
> qemu_system_powerdown_request, qemu_register_powerdown_notifier
>    for Sx->S5
> 
> and the reset mechanism for S5->S0.

Yep, I'm trying to supersede these functions
by my power chip emulation. 

> Most of these should only be used by one "power management device" on
> the board (e.g. the PIIX3 or ICH9 power management devices in hw/acpi/*)
> and distributed to the other devices via whatever buses exist on the
> real machine.
Paolo Bonzini April 9, 2013, 11:06 a.m. UTC | #7
Il 09/04/2013 10:26, li guang ha scritto:
> > qemu_system_suspend_request, qemu_register_suspend_notifier
> >    for S0->S3
> > 
> > qemu_system_wakeup_request, qemu_register_wakeup_notifier
> >    for S3->S0
> > 
> > qemu_system_powerdown_request, qemu_register_powerdown_notifier
> >    for Sx->S5
> > 
> > and the reset mechanism for S5->S0.
> 
> Yep, I'm trying to supersede these functions
> by my power chip emulation. 

Then I explained in my other message why this is wrong.  The API may
well be "bad" (if so, please explain why), but at least it is the right
tool to model this.  QEMU models abstract concepts (memory, timers,
powerdown) with APIs, not with devices.

Paolo
liguang April 10, 2013, 12:09 a.m. UTC | #8
在 2013-04-09二的 13:06 +0200,Paolo Bonzini写道:
> Il 09/04/2013 10:26, li guang ha scritto:
> > > qemu_system_suspend_request, qemu_register_suspend_notifier
> > >    for S0->S3
> > > 
> > > qemu_system_wakeup_request, qemu_register_wakeup_notifier
> > >    for S3->S0
> > > 
> > > qemu_system_powerdown_request, qemu_register_powerdown_notifier
> > >    for Sx->S5
> > > 
> > > and the reset mechanism for S5->S0.
> > 
> > Yep, I'm trying to supersede these functions
> > by my power chip emulation. 
> 
> Then I explained in my other message why this is wrong.  The API may
> well be "bad" (if so, please explain why), but at least it is the right
> tool to model this.  QEMU models abstract concepts (memory, timers,
> powerdown) with APIs, not with devices.
> 

It's probably not 'bad', just only not native, because real hardware
does not do thing that way, and also, this power chip is not purely
conceptual, it just try to integrate jobs of power control from
different platform.
of course, I can model this power chip as real hardware which exists
in specific platform.
can we just feel happy with current implementation and don't want
to try other things?  :-)
or do you consider this totally wrong for direction?
Paolo Bonzini April 10, 2013, 9:40 a.m. UTC | #9
Il 10/04/2013 02:09, li guang ha scritto:
>> > 
>> > Then I explained in my other message why this is wrong.  The API may
>> > well be "bad" (if so, please explain why), but at least it is the right
>> > tool to model this.  QEMU models abstract concepts (memory, timers,
>> > powerdown) with APIs, not with devices.
>> > 
> It's probably not 'bad', just only not native,

The point of having an API, instead of a device, is to do things in a
way that works decently for both "sides" who use it: device models on
one side, monitor and user interface on the other.

> because real hardware does not do thing that way

At some point you have to depart from real hardware, for example in the
case of a disk a virtual machine will write to a file and not to a
magnetic surface.  We try to place APIs at the exact point where this
separation happens.  Are you saying our API is placed wrong?  If so,
where would you put it?

> , and also, this power chip is not purely
> conceptual, it just try to integrate jobs of power control from
> different platform.

That's the job of an API, not a device.

> of course, I can model this power chip as real hardware which exists
> in specific platform.

What power chip do you have in mind, for example, in PCs?  Can you point
to the datasheet?  Would an implementation of that device require
changes to the API?

> can we just feel happy with current implementation and don't want
> to try other things?  :-)
> or do you consider this totally wrong for direction?

Creating an abstract device that does not match real hardware is wrong.
 Improving the API could be more interesting.

Paolo
Andreas Färber April 10, 2013, 12:09 p.m. UTC | #10
Am 10.04.2013 02:09, schrieb li guang:
> 在 2013-04-09二的 13:06 +0200,Paolo Bonzini写道:
>> Il 09/04/2013 10:26, li guang ha scritto:
>>>> qemu_system_suspend_request, qemu_register_suspend_notifier
>>>>    for S0->S3
>>>>
>>>> qemu_system_wakeup_request, qemu_register_wakeup_notifier
>>>>    for S3->S0
>>>>
>>>> qemu_system_powerdown_request, qemu_register_powerdown_notifier
>>>>    for Sx->S5
>>>>
>>>> and the reset mechanism for S5->S0.
>>>
>>> Yep, I'm trying to supersede these functions
>>> by my power chip emulation. 
>>
>> Then I explained in my other message why this is wrong.  The API may
>> well be "bad" (if so, please explain why), but at least it is the right
>> tool to model this.  QEMU models abstract concepts (memory, timers,
>> powerdown) with APIs, not with devices.
>>
> 
> It's probably not 'bad', just only not native, because real hardware
> does not do thing that way, and also, this power chip is not purely
> conceptual, it just try to integrate jobs of power control from
> different platform.
> of course, I can model this power chip as real hardware which exists
> in specific platform.

Li Guang, I think your problem is a conceptual one: QEMU does not do a
system simulation, it does a system emulation. Thus if a chip is hidden
from software and not directly accessed in terms of registers from guest
software, then we don't model it as a device but call some API functions
from where it is supposed to show effects (keyboard controller device
MemoryRegionOps, TCG instruction, monitor command, ...).

Thus we are reluctant to accept a QOM Device that is neither exposed to
the guest nor uses any QOM concepts like inheritance AFAICS. Especially
when the advantage of doing so is not well explained.

Andreas

> can we just feel happy with current implementation and don't want
> to try other things?  :-)
> or do you consider this totally wrong for direction?
guang li April 10, 2013, 1:41 p.m. UTC | #11
Yes, you're right.
The motivation is I want to implement a device
called EC which is a notion from laptop for QEMU,
EC has some main functions, like keyboard, mouse,
low-speed device control(I2C), special ACPI space,
 i8042 and ps2 mouse has been done,  power control
was left, so I tried to add this.
so, Andreas,
do you know this  chip for laptop?
any suggestions for this?
Thanks!


2013/4/10 Andreas Färber <afaerber@suse.de>

> Am 10.04.2013 02:09, schrieb li guang:
> > 在 2013-04-09二的 13:06 +0200,Paolo Bonzini写道:
> >> Il 09/04/2013 10:26, li guang ha scritto:
> >>>> qemu_system_suspend_request, qemu_register_suspend_notifier
> >>>>    for S0->S3
> >>>>
> >>>> qemu_system_wakeup_request, qemu_register_wakeup_notifier
> >>>>    for S3->S0
> >>>>
> >>>> qemu_system_powerdown_request, qemu_register_powerdown_notifier
> >>>>    for Sx->S5
> >>>>
> >>>> and the reset mechanism for S5->S0.
> >>>
> >>> Yep, I'm trying to supersede these functions
> >>> by my power chip emulation.
> >>
> >> Then I explained in my other message why this is wrong.  The API may
> >> well be "bad" (if so, please explain why), but at least it is the right
> >> tool to model this.  QEMU models abstract concepts (memory, timers,
> >> powerdown) with APIs, not with devices.
> >>
> >
> > It's probably not 'bad', just only not native, because real hardware
> > does not do thing that way, and also, this power chip is not purely
> > conceptual, it just try to integrate jobs of power control from
> > different platform.
> > of course, I can model this power chip as real hardware which exists
> > in specific platform.
>
> Li Guang, I think your problem is a conceptual one: QEMU does not do a
> system simulation, it does a system emulation. Thus if a chip is hidden
> from software and not directly accessed in terms of registers from guest
> software, then we don't model it as a device but call some API functions
> from where it is supposed to show effects (keyboard controller device
> MemoryRegionOps, TCG instruction, monitor command, ...).
>
> Thus we are reluctant to accept a QOM Device that is neither exposed to
> the guest nor uses any QOM concepts like inheritance AFAICS. Especially
> when the advantage of doing so is not well explained.
>
> Andreas
>
> > can we just feel happy with current implementation and don't want
> > to try other things?  :-)
> > or do you consider this totally wrong for direction?
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
>
Paolo Bonzini April 10, 2013, 1:52 p.m. UTC | #12
Il 10/04/2013 15:41, guang li ha scritto:
> Yes, you're right.
> The motivation is I want to implement a device
> called EC which is a notion from laptop for QEMU,
> EC has some main functions, like keyboard, mouse,
> low-speed device control(I2C), special ACPI space,
>  i8042 and ps2 mouse has been done,  power control
> was left, so I tried to add this.

Do you have a datasheet?  Have you looked at hw/acpi/?

Paolo
liguang April 11, 2013, 1:30 a.m. UTC | #13
在 2013-04-10三的 15:52 +0200,Paolo Bonzini写道:
> Il 10/04/2013 15:41, guang li ha scritto:
> > Yes, you're right.
> > The motivation is I want to implement a device
> > called EC which is a notion from laptop for QEMU,
> > EC has some main functions, like keyboard, mouse,
> > low-speed device control(I2C), special ACPI space,
> >  i8042 and ps2 mouse has been done,  power control
> > was left, so I tried to add this.
> 
> Do you have a datasheet?  Have you looked at hw/acpi/?
> 

Oh, that's a tough question, maybe you can find some
data-sheets of EC chip(WPCE775L, IT8718F, ...)
at their official website.
I can send you an old data-sheet for IT8718F
that download from website privately for avoiding
possible business issues.

Yes, I read codes at hw/acpi,
but, I can't get indication for my question.

Paolo, Thanks!
Anthony Liguori April 16, 2013, 8:45 p.m. UTC | #14
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 10/04/2013 15:41, guang li ha scritto:
>> Yes, you're right.
>> The motivation is I want to implement a device
>> called EC which is a notion from laptop for QEMU,
>> EC has some main functions, like keyboard, mouse,
>> low-speed device control(I2C), special ACPI space,
>>  i8042 and ps2 mouse has been done,  power control
>> was left, so I tried to add this.
>
> Do you have a datasheet?  Have you looked at hw/acpi/?

http://www.coreboot.org/Embedded_controller

Has more than you probably ever wanted to know about ECs.

I don't see much reason to model an EC in QEMU.  It's only visible to
firmware (at least on the PC) so there's little benefit to modeling.

Regards,

Anthony Liguori

>
> Paolo
liguang April 17, 2013, 12:07 a.m. UTC | #15
在 2013-04-16二的 15:45 -0500,Anthony Liguori写道:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > Il 10/04/2013 15:41, guang li ha scritto:
> >> Yes, you're right.
> >> The motivation is I want to implement a device
> >> called EC which is a notion from laptop for QEMU,
> >> EC has some main functions, like keyboard, mouse,
> >> low-speed device control(I2C), special ACPI space,
> >>  i8042 and ps2 mouse has been done,  power control
> >> was left, so I tried to add this.
> >
> > Do you have a datasheet?  Have you looked at hw/acpi/?
> 
> http://www.coreboot.org/Embedded_controller
> 
> Has more than you probably ever wanted to know about ECs.
> 
> I don't see much reason to model an EC in QEMU. 
> It's only visible to firmware (at least on the PC)

No, it's certainly visible to OS.
you can get more from coreboot source code.

>  so there's little benefit to modeling.

but it is very important to laptop.

> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > Paolo
>
diff mbox

Patch

diff --git a/vl.c b/vl.c
index aeed7f4..a14549e 100644
--- a/vl.c
+++ b/vl.c
@@ -171,6 +171,8 @@  int main(int argc, char **argv)
 #include "ui/qemu-spice.h"
 #include "qapi/string-input-visitor.h"
 
+#include "hw/power.h"
+
 //#define DEBUG_NET
 //#define DEBUG_SLIRP
 
@@ -4295,6 +4297,8 @@  int main(int argc, char **argv, char **envp)
 
     qdev_machine_init();
 
+    qdev_init_nofail(qdev_create(NULL, TYPE_POWER_CHIP));
+
     QEMUMachineInitArgs args = { .ram_size = ram_size,
                                  .boot_device = (boot_devices[0] == '\0') ?
                                                 machine->boot_order :