Re: [RFC][PATCH] pci-assign: Drop support for raw ioport access

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-05-29 16:41, Alex Williamson wrote:
> On Mon, 2012-05-28 at 15:03 +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> If the kernel does not support ioport access via sysfs, passthrough can
>> only help if the unlikely case that a port <= 0x3ff is provided by the
>> device. So drop this to simplify the code and to allow dropping the
>> corresponding KVM infrastructure in preparation of upstream merge.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Does anyone recall the precise use case this was introduced for? It
>> exists since day #1, so commit logs do not help.
>>
>>  hw/device-assignment.c |   20 +++-----------------
>>  1 files changed, 3 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
>> index 1daadb9..9ad5de5 100644
>> --- a/hw/device-assignment.c
>> +++ b/hw/device-assignment.c
>> @@ -245,18 +245,8 @@ static void assigned_dev_ioport_setup(PCIDevice *pci_dev, int region_num,
>>  {
>>      AssignedDevice *r_dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
>>      AssignedDevRegion *region = &r_dev->v_addrs[region_num];
>> -    int r;
>>  
>>      region->e_size = size;
>> -
>> -    if (region->region->resource_fd < 0) {
>> -        r = kvm_add_ioport_region(region->u.r_baseport, region->r_size,
>> -                                  pci_dev->qdev.hotplugged);
>> -        if (r < 0) {
>> -            fprintf(stderr, "%s: failed to enable ioport access (%m)\n",
>> -                    __func__);
>> -        }
>> -    }
>>      memory_region_init(&region->container, "assigned-dev-container", size);
>>      memory_region_init_io(&region->real_iomem, &assigned_dev_ioport_ops,
>>                            r_dev->v_addrs + region_num,
>> @@ -440,10 +430,10 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
>>                          ret);
>>                  abort();
>>              } else if (errno != EINVAL) {
>> -                fprintf(stderr, "Using raw in/out ioport access (sysfs - %s)\n",
>> -                        strerror(errno));
>> +                fprintf(stderr,
>> +                        "Kernel doesn't support ioport resource access.\n");
>>                  close(pci_dev->v_addrs[i].region->resource_fd);
>> -                pci_dev->v_addrs[i].region->resource_fd = -1;
>> +                return -1;
> 
> Jan, I think we could probably get away with making this non-fatal.
> Quite a few cards include an I/O port range that's never used.  Can we
> follow-up with a patch that just hides the I/O port BAR if we don't have
> sysfs access and print and error so we have a breadcrumb if the device
> then fails to work?  If someone is using a pre-sysfs-ioport kernel that
> would at least be a little more friendly and probably the same level of
> functionality they have now.  Thanks,

Sounds reasonable. Will have a look.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux