On 2014/10/29 5:37, Thomas Gleixner wrote:
> On Tue, 28 Oct 2014, Jiang Liu wrote:
>> +static int msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
>> +			    bool force)
>> +{
>> +	struct irq_data *parent = data->parent_data;
>> +	int ret;
>>  
>> -	msg.data &= ~MSI_DATA_VECTOR_MASK;
>> -	msg.data |= MSI_DATA_VECTOR(cfg->vector);
>> -	msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK;
>> -	msg.address_lo |= MSI_ADDR_DEST_ID(dest);
>> +	ret = parent->chip->irq_set_affinity(parent, mask, force);
>> +	/* No need to reprogram MSI registers if interrupt is remapped */
>> +	if (ret >= 0 && !msi_irq_remapped(data)) {
>> +		struct msi_msg msg;
>>  
>> -	__write_msi_msg(data->msi_desc, &msg);
>> +		__get_cached_msi_msg(data->msi_desc, &msg);
>> +		msi_update_msg(&msg, data);
>> +		__write_msi_msg(data->msi_desc, &msg);
>> +	}
> 
> I'm not too happy about the msi_irq_remapped() conditional here. It
> violates the whole concept of domain stacking somewhat.
> 
> A better separation would be to add a callback to the irq chip:
> 
>   	void (*irq_write_msi_msg)(struct irq_data *data, struct msi_desc *msi_desc, bool cached);
> 
> and change this code to:
> 
>     	if (ret >= 0)
> 	   	parent->chip->irq_write_msi_msg(parent, data->msi-desc, true);
>   
Hi Thomas,
	Thanks for your great suggestion and I have worked out a draft
patch to achieve what you want:)
	I have made following changes to irq core to get rid of remapped
irq logic from msi.c:
1) Add IRQ_SET_MASK_OK_DONE in addition to IRQ_SET_MASK_OK and
IRQ_SET_MASK_OK_NOCOPY. IRQ_SET_MASK_OK_DONE is the same as
IRQ_SET_MASK_OK for irq core and indicates to stacked irqchip that
parent irqchips have done all work and skip any handling in descendant
irqchips.
2) Add int (*irq_compose_msi_msg)(struct irq_data *data, struct msi_msg
*msg) into struct irq_chip. I'm still hesitating to return void or int
here. By returning void, irq_chip_compose_msi_msg() will be simpler,
but it loses flexibility.

With above changes to core, we could remove all knowledge of irq
remapping from msi.c and the irq remapping interfaces get simpler too.
Please refer to following patch for details. The patch passes basic
booting tests with irq remapping enabled. If it's OK, I will fold
it into the patch set.

IOAPIC runs into the same situation, but I need some more time
to find a solution:)

Regards!
Gerry