On Wed, Jun 11, 2014 at 07:37:48PM -0600, Alex Williamson wrote:
>On Thu, 2014-06-12 at 10:02 +1000, Gavin Shan wrote:
>> 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 :-)
>
>It's ok, explicit initialization doesn't hurt anything here. The series
>looks ok to me, but it depends on the header update, so it needs to wait
>for that to happen in the kernel. I provided my ack for the other
>series, but let me know if I need to push the vfio changes through my
>tree. Thanks,
>
Thanks, Alex. The kernel part should be merged firstly. All the stuff
(kernel & QEMU part) depends on Alexey's VFIO stuff. So lets wait until
Alexey's VFIO stuff gets merged. That time, I guess I probably have to
rebase and send out a new revision (with your ack of course).
Thanks,
Gavin
>> 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 = {
>> >
>>
>
>
>
