Re: [Qemu-devel] [PATCH 1/3] pc: Fix disabling of vapic for compat PC models

[Michael S. Tsirkin]

On Thu, Oct 02, 2014 at 09:27:41AM +0200, Jan Kiszka wrote:
> On 2014-09-02 17:11, Michael Roth wrote:
> > Quoting Markus Armbruster (2014-07-30 06:19:36)
> >> Paolo Bonzini <address@hidden> writes:
> >>
> >>> Il 30/07/2014 10:57, Michael S. Tsirkin ha scritto:
> >>>> On Wed, Jul 30, 2014 at 09:01:59AM +0200, Jan Kiszka wrote:
> >>>>> We used to be able to address both the QEMU and the KVM APIC via "apic".
> >>>>> This doesn't work anymore. So we need to use their parent class to turn
> >>>>> off the vapic on machines that should not expose them.
> >>>>>
> >>>>> Signed-off-by: Jan Kiszka <address@hidden>
> >>>>
> >>>>
> >>>>
> >>>> OK so this is intended for 2.2?
> >>
> >> If yes, we should cc: qemu-stable.
> > 
> > Ping for stable 2.1.1, freeze is on Wednesday
> 
> Lost track of this: was I supposed to provide anything different, or did
> this just fall under the table?
> 
> Jan

Yes, I think Michael expected an ACK for stable.
Oh well.
Would you like me to apply as is, or to rework this to avoid duplication
of string names?

> > 
> >>
> >>>> In that case, how about creating a macro with type name,
> >>>> and using that? This way things don't break if we rename
> >>>> something again.
> >>>
> >>> Don't we have warnings for that now?
> >>
> >> Warnings don't help much in cases like this: "apic" still exists and has
> >> the property, it's just not the device we want.  Macros aren't
> >> foolproof, either.
> >>
> >>>>> ---
> >>>>>  hw/i386/pc_piix.c | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >>>>> index 9694f88..73ba77d 100644
> >>>>> --- a/hw/i386/pc_piix.c
> >>>>> +++ b/hw/i386/pc_piix.c
> >>>>> @@ -645,7 +645,7 @@ static QEMUMachine pc_machine_v1_1 = {
> >>>>>              .property = "class",\
> >>>>>              .value    = stringify(PCI_CLASS_MEMORY_RAM),\
> >>>>>          },{\
> >>>>> -            .driver   = "apic",\
> >>>>> +            .driver   = "apic-common",\
> >>>>>              .property = "vapic",\
> >>>>>              .value    = "off",\
> >>>>>          },{\
> >>>>> -- 
> >>>>> 1.8.1.1.298.ge7eed54
> >>
> >> You could use TYPE_APIC_COMMON here.  Including
> >> "hw/i386/apic_internal.h" for it would be not so nice, though.
> > 
> 
> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SES-DE
> Corporate Competence Center Embedded Linux