On Wed, Jun 11, 2014 at 02:26:51PM -0600, Alex Williamson wrote:
>On Tue, 2014-06-10 at 12:03 +1000, Gavin Shan wrote:
>> The patch implements sPAPRPHBClass::eeh_handler so that the
>> EEH RTAS requests can be routed to VFIO for further handling.
>>
>> Signed-off-by: Gavin Shan <gws...@linux.vnet.ibm.com>
>> ---
>> hw/ppc/spapr_pci_vfio.c | 56
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 56 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
>> index 592d6a4..9750cf0 100644
>> --- a/hw/ppc/spapr_pci_vfio.c
>> +++ b/hw/ppc/spapr_pci_vfio.c
>> @@ -85,6 +85,61 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState
>> *sphb, Error **errp)
>> spapr_tce_get_iommu(tcet));
>> }
>>
>> +static int spapr_phb_vfio_eeh_handler(sPAPRPHBState *sphb, int req, int opt)
>> +{
>> + sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
>> + struct vfio_eeh_pe_op op = { .argsz = sizeof(op), .flags = 0 };
>
>FWIW, flags = 0 isn't actually necessary. I'm sure someone here can
>quote the C spec, but it's my understanding that if any field of a
>structure is initialized, the remaining fields are zero initialized.
>vfio.c has a mix of initializations depending on whether using an
>explicit value for flags adds to the code clarity.
>
Yes, but it's not harmful. Please let me know if you want me to remove
it :-)
I had a very quick experiment on x86
and Power Linux with following tiny program and the result is just
what you think:
With "struct test foo" in func2():
func2: foo.a=0xffffffff, foo.b=0xffffffff
with "static struct test foo" in func2(). Here's the explaining about
this: section 2.4.2.3 of
http://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Initializing-Structure-Members
func2: foo.a=0x00000000, foo.b=0x00000000
with "struct test foo = { .a = 0 }" in func2().
func2: foo.a=0x00000000, foo.b=0x00000000
With "struct test foo = { 0 }" in func2():
func2: foo.a=0x00000000, foo.b=0x00000000
---
#include <stdio.h>
struct test {
int a;
int b;
};
static func1(void)
{
int var[1000];
int i;
for (i = 0; i < 1000; i++)
var[i] = 0xffffffff;
}
static func2(void)
{
struct test foo;
printf("%s: foo.a=0x%08x, foo.b=0x%08x\n",
__func__, foo.a, foo.b);
}
int main(int argc, char **argv)
{
func1();
func2();
return 0;
}
Thanks,
Gavin
>> + int cmd;
>> +
>> + switch (req) {
>> + case RTAS_EEH_REQ_SET_OPTION:
>> + switch (opt) {
>> + case RTAS_EEH_DISABLE:
>> + cmd = VFIO_EEH_PE_DISABLE;
>> + break;
>> + case RTAS_EEH_ENABLE:
>> + cmd = VFIO_EEH_PE_ENABLE;
>> + break;
>> + case RTAS_EEH_THAW_IO:
>> + cmd = VFIO_EEH_PE_UNFREEZE_IO;
>> + break;
>> + case RTAS_EEH_THAW_DMA:
>> + cmd = VFIO_EEH_PE_UNFREEZE_DMA;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + break;
>> + case RTAS_EEH_REQ_GET_STATE:
>> + cmd = VFIO_EEH_PE_GET_STATE;
>> + break;
>> + case RTAS_EEH_REQ_RESET:
>> + switch (opt) {
>> + case RTAS_SLOT_RESET_DEACTIVATE:
>> + cmd = VFIO_EEH_PE_RESET_DEACTIVATE;
>> + break;
>> + case RTAS_SLOT_RESET_HOT:
>> + cmd = VFIO_EEH_PE_RESET_HOT;
>> + break;
>> + case RTAS_SLOT_RESET_FUNDAMENTAL:
>> + cmd = VFIO_EEH_PE_RESET_FUNDAMENTAL;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + break;
>> + case RTAS_EEH_REQ_CONFIGURE:
>> + cmd = VFIO_EEH_PE_CONFIGURE;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + op.op = cmd;
>> + return vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
>> + VFIO_EEH_PE_OP, &op);
>> +}
>> +
>> static void spapr_phb_vfio_reset(DeviceState *qdev)
>> {
>> /* Do nothing */
>> @@ -98,6 +153,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass,
>> void *data)
>> dc->props = spapr_phb_vfio_properties;
>> dc->reset = spapr_phb_vfio_reset;
>> spc->finish_realize = spapr_phb_vfio_finish_realize;
>> + spc->eeh_handler = spapr_phb_vfio_eeh_handler;
>> }
>>
>> static const TypeInfo spapr_phb_vfio_info = {
>
