Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer
On 17/07/2019 04:08, David Gibson wrote:
> On Mon, Jul 15, 2019 at 05:45:38PM +0200, Cédric Le Goater wrote:
>> On 12/07/2019 03:15, David Gibson wrote:
>>> On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote:
On 03/07/2019 04:07, David Gibson wrote:
> On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
>> This is to perform lookups in the NVT table when a vCPU is dispatched
>> and possibly resend interrupts.
>
> I'm slightly confused by this one. Aren't there multiple router
> objects, each of which can deliver to any thread? In which case what
> router object is associated with a specific TCTX?
when a vCPU is dispatched on a HW thread, the hypervisor does a store
on the CAM line to store the VP id. At that time, it checks the IPB in
the associated NVT structure and notifies the thread if an interrupt is
pending.
We need to do a NVT lookup, just like the presenter in HW, hence the
router pointer. You should look at the following patch which clarifies
the resend sequence.
>>>
>>> Hm, ok.
>>>
>> Future XIVE chip will use a different class for the model of the
>> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
>
> This seems odd to me, shouldn't it be an interface pointer or
> something in that case?
I have duplicated most of the XIVE models for P10 because the internal
structures have changed. I managed to keep the XiveSource and XiveTCTX
but we now have a Xive10Router, this is the reason why.
>>>
>>> Right, but XiveRouter and Xive10Router must have something in common
>>> if they can both be used here. Usually that's expressed as a shared
>>> QOM interface - in which case you can use a pointer to the interface,
>>> rathe than using Object * which kind of implies *anything* can go
>>> here.
>>
>> Yeah. I also think it would be better to have a common base object but
>> the class don't have much in common. Here is what I have for now :
>
> Uh.. QOM interfaces don't require there to be a common base object,
> only common methods.
It's not a QOM interface today because it already uses an interface.
>>
>> P9:
>>
>> typedef struct XiveRouterClass {
>> SysBusDeviceClass parent;
>>
>> /* XIVE table accessors */
>> int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>>XiveEAS *eas);
>> int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>>XiveEND *end);
>> int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>> XiveEND *end, uint8_t word_number);
>> int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>XiveNVT *nvt);
>> int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>> XiveNVT *nvt, uint8_t word_number);
>> XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
>> uint8_t (*get_block_id)(XiveRouter *xrtr);
>> } XiveRouterClass;
>>
>> and P10:
>>
>> typedef struct Xive10RouterClass {
>> SysBusDeviceClass parent;
>>
>> /* XIVE table accessors */
>> int (*get_eas)(Xive10Router *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>>Xive10EAS *eas);
>> int (*get_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
>>Xive10END *end);
>> int (*write_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
>> Xive10END *end, uint8_t word_number);
>> int (*get_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>Xive10NVP *nvt);
>> int (*write_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>> Xive10NVP *nvt, uint8_t word_number);
>> XiveTCTX *(*get_tctx)(Xive10Router *xrtr, CPUState *cs);
>> uint8_t (*get_block_id)(XiveRouter *xrtr);
>> uint32_t (*get_config)(Xive10Router *xrtr);
>> } Xive10RouterClass;
>>
>> Only get_tctx() is common.
>>
>> The XIVE structures (END, NV*) used by the routing algo have changed a lot.
>> Even the presenter has changed, because all the CAM lines have a slightly
>> different format.
>>
>> C.
>>
>>
>
Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer
On Mon, Jul 15, 2019 at 05:45:38PM +0200, Cédric Le Goater wrote:
> On 12/07/2019 03:15, David Gibson wrote:
> > On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote:
> >> On 03/07/2019 04:07, David Gibson wrote:
> >>> On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
> This is to perform lookups in the NVT table when a vCPU is dispatched
> and possibly resend interrupts.
> >>>
> >>> I'm slightly confused by this one. Aren't there multiple router
> >>> objects, each of which can deliver to any thread? In which case what
> >>> router object is associated with a specific TCTX?
> >>
> >> when a vCPU is dispatched on a HW thread, the hypervisor does a store
> >> on the CAM line to store the VP id. At that time, it checks the IPB in
> >> the associated NVT structure and notifies the thread if an interrupt is
> >> pending.
> >>
> >> We need to do a NVT lookup, just like the presenter in HW, hence the
> >> router pointer. You should look at the following patch which clarifies
> >> the resend sequence.
> >
> > Hm, ok.
> >
> Future XIVE chip will use a different class for the model of the
> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
> >>>
> >>> This seems odd to me, shouldn't it be an interface pointer or
> >>> something in that case?
> >>
> >> I have duplicated most of the XIVE models for P10 because the internal
> >> structures have changed. I managed to keep the XiveSource and XiveTCTX
> >> but we now have a Xive10Router, this is the reason why.
> >
> > Right, but XiveRouter and Xive10Router must have something in common
> > if they can both be used here. Usually that's expressed as a shared
> > QOM interface - in which case you can use a pointer to the interface,
> > rathe than using Object * which kind of implies *anything* can go
> > here.
>
> Yeah. I also think it would be better to have a common base object but
> the class don't have much in common. Here is what I have for now :
Uh.. QOM interfaces don't require there to be a common base object,
only common methods.
>
> P9:
>
> typedef struct XiveRouterClass {
> SysBusDeviceClass parent;
>
> /* XIVE table accessors */
> int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>XiveEAS *eas);
> int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>XiveEND *end);
> int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
> XiveEND *end, uint8_t word_number);
> int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>XiveNVT *nvt);
> int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
> XiveNVT *nvt, uint8_t word_number);
> XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
> uint8_t (*get_block_id)(XiveRouter *xrtr);
> } XiveRouterClass;
>
> and P10:
>
> typedef struct Xive10RouterClass {
> SysBusDeviceClass parent;
>
> /* XIVE table accessors */
> int (*get_eas)(Xive10Router *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>Xive10EAS *eas);
> int (*get_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
>Xive10END *end);
> int (*write_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
> Xive10END *end, uint8_t word_number);
> int (*get_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>Xive10NVP *nvt);
> int (*write_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
> Xive10NVP *nvt, uint8_t word_number);
> XiveTCTX *(*get_tctx)(Xive10Router *xrtr, CPUState *cs);
> uint8_t (*get_block_id)(XiveRouter *xrtr);
> uint32_t (*get_config)(Xive10Router *xrtr);
> } Xive10RouterClass;
>
> Only get_tctx() is common.
>
> The XIVE structures (END, NV*) used by the routing algo have changed a lot.
> Even the presenter has changed, because all the CAM lines have a slightly
> different format.
>
> C.
>
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer
On 12/07/2019 03:15, David Gibson wrote:
> On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote:
>> On 03/07/2019 04:07, David Gibson wrote:
>>> On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
This is to perform lookups in the NVT table when a vCPU is dispatched
and possibly resend interrupts.
>>>
>>> I'm slightly confused by this one. Aren't there multiple router
>>> objects, each of which can deliver to any thread? In which case what
>>> router object is associated with a specific TCTX?
>>
>> when a vCPU is dispatched on a HW thread, the hypervisor does a store
>> on the CAM line to store the VP id. At that time, it checks the IPB in
>> the associated NVT structure and notifies the thread if an interrupt is
>> pending.
>>
>> We need to do a NVT lookup, just like the presenter in HW, hence the
>> router pointer. You should look at the following patch which clarifies
>> the resend sequence.
>
> Hm, ok.
>
Future XIVE chip will use a different class for the model of the
interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
>>>
>>> This seems odd to me, shouldn't it be an interface pointer or
>>> something in that case?
>>
>> I have duplicated most of the XIVE models for P10 because the internal
>> structures have changed. I managed to keep the XiveSource and XiveTCTX
>> but we now have a Xive10Router, this is the reason why.
>
> Right, but XiveRouter and Xive10Router must have something in common
> if they can both be used here. Usually that's expressed as a shared
> QOM interface - in which case you can use a pointer to the interface,
> rathe than using Object * which kind of implies *anything* can go
> here.
Yeah. I also think it would be better to have a common base object but
the class don't have much in common. Here is what I have for now :
P9:
typedef struct XiveRouterClass {
SysBusDeviceClass parent;
/* XIVE table accessors */
int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
XiveEAS *eas);
int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
XiveEND *end);
int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
XiveEND *end, uint8_t word_number);
int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
XiveNVT *nvt);
int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
XiveNVT *nvt, uint8_t word_number);
XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
uint8_t (*get_block_id)(XiveRouter *xrtr);
} XiveRouterClass;
and P10:
typedef struct Xive10RouterClass {
SysBusDeviceClass parent;
/* XIVE table accessors */
int (*get_eas)(Xive10Router *xrtr, uint8_t eas_blk, uint32_t eas_idx,
Xive10EAS *eas);
int (*get_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
Xive10END *end);
int (*write_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
Xive10END *end, uint8_t word_number);
int (*get_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
Xive10NVP *nvt);
int (*write_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
Xive10NVP *nvt, uint8_t word_number);
XiveTCTX *(*get_tctx)(Xive10Router *xrtr, CPUState *cs);
uint8_t (*get_block_id)(XiveRouter *xrtr);
uint32_t (*get_config)(Xive10Router *xrtr);
} Xive10RouterClass;
Only get_tctx() is common.
The XIVE structures (END, NV*) used by the routing algo have changed a lot.
Even the presenter has changed, because all the CAM lines have a slightly
different format.
C.
Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer
On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote:
> On 03/07/2019 04:07, David Gibson wrote:
> > On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
> >> This is to perform lookups in the NVT table when a vCPU is dispatched
> >> and possibly resend interrupts.
> >
> > I'm slightly confused by this one. Aren't there multiple router
> > objects, each of which can deliver to any thread? In which case what
> > router object is associated with a specific TCTX?
>
> when a vCPU is dispatched on a HW thread, the hypervisor does a store
> on the CAM line to store the VP id. At that time, it checks the IPB in
> the associated NVT structure and notifies the thread if an interrupt is
> pending.
>
> We need to do a NVT lookup, just like the presenter in HW, hence the
> router pointer. You should look at the following patch which clarifies
> the resend sequence.
Hm, ok.
> >> Future XIVE chip will use a different class for the model of the
> >> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
> >
> > This seems odd to me, shouldn't it be an interface pointer or
> > something in that case?
>
> I have duplicated most of the XIVE models for P10 because the internal
> structures have changed. I managed to keep the XiveSource and XiveTCTX
> but we now have a Xive10Router, this is the reason why.
Right, but XiveRouter and Xive10Router must have something in common
if they can both be used here. Usually that's expressed as a shared
QOM interface - in which case you can use a pointer to the interface,
rathe than using Object * which kind of implies *anything* can go
here.
>
> If I was to duplicate XiveTCTX also, I will switch it back to a XiveRouter
> pointer in the P9 version.
>
> C.
>
>
> >> Signed-off-by: Cédric Le Goater
> >
> >> ---
> >> include/hw/ppc/xive.h | 4 +++-
> >> hw/intc/xive.c| 11 ++-
> >> hw/ppc/pnv.c | 2 +-
> >> hw/ppc/spapr_irq.c| 2 +-
> >> 4 files changed, 15 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> >> index d922524982d3..b764e1e4e6d4 100644
> >> --- a/include/hw/ppc/xive.h
> >> +++ b/include/hw/ppc/xive.h
> >> @@ -321,6 +321,8 @@ typedef struct XiveTCTX {
> >> qemu_irqos_output;
> >>
> >> uint8_t regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
> >> +
> >> +Object *xrtr;
> >> } XiveTCTX;
> >>
> >> /*
> >> @@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset,
> >> uint64_t value,
> >> uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
> >>
> >> void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
> >> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
> >> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
> >>
> >> static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t
> >> nvt_idx)
> >> {
> >> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> >> index f7ba1c3b622f..56700681884f 100644
> >> --- a/hw/intc/xive.c
> >> +++ b/hw/intc/xive.c
> >> @@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error
> >> **errp)
> >> Object *obj;
> >> Error *local_err = NULL;
> >>
> >> +obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err);
> >> +if (!obj) {
> >> +error_propagate(errp, local_err);
> >> +error_prepend(errp, "required link 'xrtr' not found: ");
> >> +return;
> >> +}
> >> +tctx->xrtr = obj;
> >> +
> >> obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
> >> if (!obj) {
> >> error_propagate(errp, local_err);
> >> @@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = {
> >> .class_init= xive_tctx_class_init,
> >> };
> >>
> >> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
> >> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp)
> >> {
> >> Error *local_err = NULL;
> >> Object *obj;
> >> @@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter
> >> *xrtr, Error **errp)
> >> object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
> >> object_unref(obj);
> >> object_property_add_const_link(obj, "cpu", cpu, &error_abort);
> >> +object_property_add_const_link(obj, "xrtr", xrtr, &error_abort);
> >> object_property_set_bool(obj, true, "realized", &local_err);
> >> if (local_err) {
> >> goto error;
> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> >> index b87e01e5b925..11916dc273c2 100644
> >> --- a/hw/ppc/pnv.c
> >> +++ b/hw/ppc/pnv.c
> >> @@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip,
> >> PowerPCCPU *cpu,
> >> * controller object is initialized afterwards. Hopefully, it's
> >> * only used at runtime.
> >> */
> >> -obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive),
> >> &local_err);
> >> +obj = xive_tctx_cr
Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer
On 03/07/2019 04:07, David Gibson wrote:
> On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
>> This is to perform lookups in the NVT table when a vCPU is dispatched
>> and possibly resend interrupts.
>
> I'm slightly confused by this one. Aren't there multiple router
> objects, each of which can deliver to any thread? In which case what
> router object is associated with a specific TCTX?
when a vCPU is dispatched on a HW thread, the hypervisor does a store
on the CAM line to store the VP id. At that time, it checks the IPB in
the associated NVT structure and notifies the thread if an interrupt is
pending.
We need to do a NVT lookup, just like the presenter in HW, hence the
router pointer. You should look at the following patch which clarifies
the resend sequence.
>> Future XIVE chip will use a different class for the model of the
>> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
>
> This seems odd to me, shouldn't it be an interface pointer or
> something in that case?
I have duplicated most of the XIVE models for P10 because the internal
structures have changed. I managed to keep the XiveSource and XiveTCTX
but we now have a Xive10Router, this is the reason why.
If I was to duplicate XiveTCTX also, I will switch it back to a XiveRouter
pointer in the P9 version.
C.
>> Signed-off-by: Cédric Le Goater
>
>> ---
>> include/hw/ppc/xive.h | 4 +++-
>> hw/intc/xive.c| 11 ++-
>> hw/ppc/pnv.c | 2 +-
>> hw/ppc/spapr_irq.c| 2 +-
>> 4 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index d922524982d3..b764e1e4e6d4 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -321,6 +321,8 @@ typedef struct XiveTCTX {
>> qemu_irqos_output;
>>
>> uint8_t regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
>> +
>> +Object *xrtr;
>> } XiveTCTX;
>>
>> /*
>> @@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset,
>> uint64_t value,
>> uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
>>
>> void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
>> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
>> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
>>
>> static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>> {
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index f7ba1c3b622f..56700681884f 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error
>> **errp)
>> Object *obj;
>> Error *local_err = NULL;
>>
>> +obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err);
>> +if (!obj) {
>> +error_propagate(errp, local_err);
>> +error_prepend(errp, "required link 'xrtr' not found: ");
>> +return;
>> +}
>> +tctx->xrtr = obj;
>> +
>> obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
>> if (!obj) {
>> error_propagate(errp, local_err);
>> @@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = {
>> .class_init= xive_tctx_class_init,
>> };
>>
>> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp)
>> {
>> Error *local_err = NULL;
>> Object *obj;
>> @@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr,
>> Error **errp)
>> object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
>> object_unref(obj);
>> object_property_add_const_link(obj, "cpu", cpu, &error_abort);
>> +object_property_add_const_link(obj, "xrtr", xrtr, &error_abort);
>> object_property_set_bool(obj, true, "realized", &local_err);
>> if (local_err) {
>> goto error;
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index b87e01e5b925..11916dc273c2 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip,
>> PowerPCCPU *cpu,
>> * controller object is initialized afterwards. Hopefully, it's
>> * only used at runtime.
>> */
>> -obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive),
>> &local_err);
>> +obj = xive_tctx_create(OBJECT(cpu), OBJECT(&chip9->xive), &local_err);
>> if (local_err) {
>> error_propagate(errp, local_err);
>> return;
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index b2b01e850de8..5b3c3c50967b 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -353,7 +353,7 @@ static void
>> spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr,
>> Object *obj;
>> SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>>
>> -obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive),
>> &local_err);
>> +obj = xive_tctx_cr
Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer
On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
> This is to perform lookups in the NVT table when a vCPU is dispatched
> and possibly resend interrupts.
I'm slightly confused by this one. Aren't there multiple router
objects, each of which can deliver to any thread? In which case what
router object is associated with a specific TCTX?
> Future XIVE chip will use a different class for the model of the
> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
This seems odd to me, shouldn't it be an interface pointer or
something in that case?
> Signed-off-by: Cédric Le Goater
> ---
> include/hw/ppc/xive.h | 4 +++-
> hw/intc/xive.c| 11 ++-
> hw/ppc/pnv.c | 2 +-
> hw/ppc/spapr_irq.c| 2 +-
> 4 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index d922524982d3..b764e1e4e6d4 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -321,6 +321,8 @@ typedef struct XiveTCTX {
> qemu_irqos_output;
>
> uint8_t regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
> +
> +Object *xrtr;
> } XiveTCTX;
>
> /*
> @@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset,
> uint64_t value,
> uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
>
> void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
>
> static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
> {
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index f7ba1c3b622f..56700681884f 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error
> **errp)
> Object *obj;
> Error *local_err = NULL;
>
> +obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err);
> +if (!obj) {
> +error_propagate(errp, local_err);
> +error_prepend(errp, "required link 'xrtr' not found: ");
> +return;
> +}
> +tctx->xrtr = obj;
> +
> obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
> if (!obj) {
> error_propagate(errp, local_err);
> @@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = {
> .class_init= xive_tctx_class_init,
> };
>
> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp)
> {
> Error *local_err = NULL;
> Object *obj;
> @@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr,
> Error **errp)
> object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
> object_unref(obj);
> object_property_add_const_link(obj, "cpu", cpu, &error_abort);
> +object_property_add_const_link(obj, "xrtr", xrtr, &error_abort);
> object_property_set_bool(obj, true, "realized", &local_err);
> if (local_err) {
> goto error;
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index b87e01e5b925..11916dc273c2 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip,
> PowerPCCPU *cpu,
> * controller object is initialized afterwards. Hopefully, it's
> * only used at runtime.
> */
> -obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive),
> &local_err);
> +obj = xive_tctx_create(OBJECT(cpu), OBJECT(&chip9->xive), &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index b2b01e850de8..5b3c3c50967b 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -353,7 +353,7 @@ static void
> spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr,
> Object *obj;
> SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>
> -obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive),
> &local_err);
> +obj = xive_tctx_create(OBJECT(cpu), OBJECT(spapr->xive), &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer
This is to perform lookups in the NVT table when a vCPU is dispatched
and possibly resend interrupts.
Future XIVE chip will use a different class for the model of the
interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
Signed-off-by: Cédric Le Goater
---
include/hw/ppc/xive.h | 4 +++-
hw/intc/xive.c| 11 ++-
hw/ppc/pnv.c | 2 +-
hw/ppc/spapr_irq.c| 2 +-
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index d922524982d3..b764e1e4e6d4 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -321,6 +321,8 @@ typedef struct XiveTCTX {
qemu_irqos_output;
uint8_t regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
+
+Object *xrtr;
} XiveTCTX;
/*
@@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset,
uint64_t value,
uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
-Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
+Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
{
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index f7ba1c3b622f..56700681884f 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error
**errp)
Object *obj;
Error *local_err = NULL;
+obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err);
+if (!obj) {
+error_propagate(errp, local_err);
+error_prepend(errp, "required link 'xrtr' not found: ");
+return;
+}
+tctx->xrtr = obj;
+
obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
if (!obj) {
error_propagate(errp, local_err);
@@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = {
.class_init= xive_tctx_class_init,
};
-Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
+Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp)
{
Error *local_err = NULL;
Object *obj;
@@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr,
Error **errp)
object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
object_unref(obj);
object_property_add_const_link(obj, "cpu", cpu, &error_abort);
+object_property_add_const_link(obj, "xrtr", xrtr, &error_abort);
object_property_set_bool(obj, true, "realized", &local_err);
if (local_err) {
goto error;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index b87e01e5b925..11916dc273c2 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip,
PowerPCCPU *cpu,
* controller object is initialized afterwards. Hopefully, it's
* only used at runtime.
*/
-obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
+obj = xive_tctx_create(OBJECT(cpu), OBJECT(&chip9->xive), &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index b2b01e850de8..5b3c3c50967b 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -353,7 +353,7 @@ static void
spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr,
Object *obj;
SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
-obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive), &local_err);
+obj = xive_tctx_create(OBJECT(cpu), OBJECT(spapr->xive), &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
--
2.21.0
