[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-
From: |
Nikunj A Dadhania |
Subject: |
Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class |
Date: |
Tue, 20 Sep 2016 11:59:59 +0530 |
User-agent: |
Notmuch/0.21 (https://notmuchmail.org) Emacs/25.0.94.1 (x86_64-redhat-linux-gnu) |
Cédric Le Goater <address@hidden> writes:
> On 09/20/2016 08:02 AM, Nikunj A Dadhania wrote:
>> Cédric Le Goater <address@hidden> writes:
>>
>>> On 09/19/2016 08:29 AM, Nikunj A Dadhania wrote:
>>>> From: Benjamin Herrenschmidt <address@hidden>
>>>>
>>>> The existing implementation remains same and ics-base is introduced. The
>>>> type name "ics" is retained, and all the related functions renamed as
>>>> ics_simple_*
>>>>
>>>> This will allow different implementations for the source controllers
>>>> such as the MSI support of PHB3 on Power8 which uses in-memory state
>>>> tables for example.
>>>
>>> A couple of issues below regarding the class helpers,
>>>
>>>> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
>>>> Signed-off-by: Nikunj A Dadhania <address@hidden>
>>>> ---
>>>> hw/intc/trace-events | 10 ++--
>>>> hw/intc/xics.c | 143
>>>> +++++++++++++++++++++++++++++++-------------------
>>>> hw/intc/xics_kvm.c | 10 ++--
>>>> hw/intc/xics_spapr.c | 28 +++++-----
>>>> include/hw/ppc/xics.h | 23 +++++---
>>>> 5 files changed, 131 insertions(+), 83 deletions(-)
>>>>
>>>> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
>>>> index aa342a8..a367b46 100644
>>>> --- a/hw/intc/trace-events
>>>> +++ b/hw/intc/trace-events
>>>> @@ -50,12 +50,12 @@ xics_icp_accept(uint32_t old_xirr, uint32_t new_xirr)
>>>> "icp_accept: XIRR %#"PRIx3
>>>> xics_icp_eoi(int server, uint32_t xirr, uint32_t new_xirr) "icp_eoi:
>>>> server %d given XIRR %#"PRIx32" new XIRR %#"PRIx32
>>>> xics_icp_irq(int server, int nr, uint8_t priority) "cpu %d trying to
>>>> deliver irq %#"PRIx32" priority %#x"
>>>> xics_icp_raise(uint32_t xirr, uint8_t pending_priority) "raising IRQ new
>>>> XIRR=%#x new pending priority=%#x"
>>>> -xics_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq %#x]"
>>>> +xics_ics_simple_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d
>>>> [irq %#x]"
>>>> xics_masked_pending(void) "set_irq_msi: masked pending"
>>>> -xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
>>>> -xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority)
>>>> "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
>>>> -xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
>>>> -xics_ics_eoi(int nr) "ics_eoi: irq %#x"
>>>> +xics_ics_simple_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d
>>>> [irq %#x]"
>>>> +xics_ics_simple_write_xive(int nr, int srcno, int server, uint8_t
>>>> priority) "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
>>>> +xics_ics_simple_reject(int nr, int srcno) "reject irq %#x [src %d]"
>>>> +xics_ics_simple_eoi(int nr) "ics_eoi: irq %#x"
>>>> xics_alloc(int irq) "irq %d"
>>>> xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d,
>>>> %d irqs, lsi=%d, alignnum %d"
>>>> xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d
>>>> irqs"
>>>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>>>> index c7901c4..b15751e 100644
>>>> --- a/hw/intc/xics.c
>>>> +++ b/hw/intc/xics.c
>>>> @@ -113,7 +113,7 @@ void xics_add_ics(XICSState *xics)
>>>> {
>>>> ICSState *ics;
>>>>
>>>> - ics = ICS(object_new(TYPE_ICS));
>>>> + ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE));
>>>> object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
>>>> ics->xics = xics;
>>>> QLIST_INSERT_HEAD(&xics->ics, ics, list);
>>>> @@ -224,9 +224,32 @@ static const TypeInfo xics_common_info = {
>>>> #define XISR(ss) (((ss)->xirr) & XISR_MASK)
>>>> #define CPPR(ss) (((ss)->xirr) >> 24)
>>>>
>>>> -static void ics_reject(ICSState *ics, int nr);
>>>> -static void ics_resend(ICSState *ics, int server);
>>>> -static void ics_eoi(ICSState *ics, int nr);
>>>> +static void ics_reject(ICSState *ics, uint32_t nr)
>>>> +{
>>>> + ICSStateClass *k = ICS_GET_CLASS(ics);
>>>
>>> Shouldn't that be ICS_BASE_GET_CLASS()
>>
>> The class hierarchy is something like this:
>>
>> ICS_BASE -> ICS_SIMPLE -> ICS_KVM
>
> yes. but if we called ics_* with an instance of an ics class which is
> not a ICS_SIMPLE class that will break.
Correct
> ICSStateClass is the baseclass, whenever we call methods on a ICSState*
> object, we should use the method defines in ICSStateClass.
Hmm, in that case I need to initialize base class methods in
instance_init of ics_simple
>> We have an instance init for ICS_SIMPLE where we set these pointers.
>>
>>>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>>>> index a7a1e54..2231f2a 100644
>>>> --- a/include/hw/ppc/xics.h
>>>> +++ b/include/hw/ppc/xics.h
>>>> @@ -119,22 +119,29 @@ struct ICPState {
>>>> bool cap_irq_xics_enabled;
>>>> };
>>>>
>>>> -#define TYPE_ICS "ics"
>>>> -#define ICS(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS)
>>>> +#define TYPE_ICS_BASE "ics-base"
>>>> +#define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE)
>>>
>>> #define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_BASE)
>>
>> Yes, that is a bug. Will correct.
>>
>>>
>>>> -#define TYPE_KVM_ICS "icskvm"
>>>> -#define KVM_ICS(obj) OBJECT_CHECK(ICSState, (obj), TYPE_KVM_ICS)
>>>> +/* Retain ics for sPAPR for migration from existing sPAPR guests */
>>>> +#define TYPE_ICS_SIMPLE "ics"
>>>> +#define ICS_SIMPLE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE)
>>>> +
>>>> +#define TYPE_ICS_KVM "icskvm"
>>>> +#define ICS_KVM(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_KVM)
>>>>
>>>> #define ICS_CLASS(klass) \
>>>> - OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS)
>>>> + OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS_SIMPLE)
>>>> #define ICS_GET_CLASS(obj) \
>>>> - OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS)
>>>> + OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS_SIMPLE)
>>>
>>> #define ICS_BASE_CLASS(klass) \
>>> OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS_BASE)
>>> #define ICS_BASE_GET_CLASS(obj) \
>>> OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS_BASE)
>>
>> As explained above, not needed.
>
> We will with powernv which defines a new ics type for the phb3 msis.
Right, will change it.
Regards
Nikunj
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 3/9] ppc/xics: Make the ICSState a list, (continued)
- [Qemu-devel] [PATCH v4 8/9] ppc/xics: Add xics to the monitor "info pic" command, Nikunj A Dadhania, 2016/09/19
- [Qemu-devel] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Nikunj A Dadhania, 2016/09/19
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Cédric Le Goater, 2016/09/19
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Cédric Le Goater, 2016/09/19
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Nikunj A Dadhania, 2016/09/20
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Cédric Le Goater, 2016/09/20
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class,
Nikunj A Dadhania <=
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Cédric Le Goater, 2016/09/20
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Nikunj A Dadhania, 2016/09/20
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Cédric Le Goater, 2016/09/20
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Nikunj A Dadhania, 2016/09/20
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Cédric Le Goater, 2016/09/20
- Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class, Nikunj A Dadhania, 2016/09/20
[Qemu-devel] [PATCH v4 7/9] ppc/xics: Add "native" XICS subclass, Nikunj A Dadhania, 2016/09/19