|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 1/3] xen/igd: get PCH info from host sysfs
Sorry I am not the maintainer/reviewer of Xen IGD passthrough, just out
of my personal interest
On 2026-07-10 02:35, Chuck Zmudzinski wrote:
> The igd_combo_id_infos[] data is more than 10 years
> out of date with many Intel IGD devices missing from
> igd_combo_id_infos[]. This means that many devices
> that could be supported will not work with the
> current implementation.
>
> For newer devices not listed in igd_combo_id_infos[],
> get infos from the host sysfs to enable support for the
> newer devices not listed in igd_combo_id_infos[].
>
> Introduce the helper function xen_pt_get_host_pch_info
> to facilitate getting the necessary information from
> sysfs.
>
> Also, use errp in xen_igd_passthrough_isa_bridge_create
> to set errors from xen_pt_get_host_pch_info.
>
> Signed-off-by: Chuck Zmudzinski <brchuckz@xxxxxxx>
> ---
> Changes in v2:
> - call error_setg* after closing files instead of before closing
> files
> - in last line of commit message change "to propagate errors" to
> "to set errors"
> - add stable to Cc list
>
> Changes in v3:
> - whitespace fix at line 380 of xen_pt_graphics.c
> - fix Cc address for qemu-stable
>
> hw/xen/xen_pt.c | 2 +-
> hw/xen/xen_pt_graphics.c | 82 ++++++++++++++++++++++++++++++++++++++--
> include/hw/xen/xen_igd.h | 3 +-
> 3 files changed, 82 insertions(+), 5 deletions(-)
>
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index 0fe9c0a..474606e 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -867,7 +867,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
> }
>
> /* Register ISA bridge for passthrough GFX. */
> - xen_igd_passthrough_isa_bridge_create(s, &s->real_device);
> + xen_igd_passthrough_isa_bridge_create(s, &s->real_device, errp);
The `errp` need to be handled here if any error occurs.
> }
>
> /* Handle real device's MMIO/PIO BARs */
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index 7df9344..2ef941e 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -376,8 +376,75 @@ static void pt_graphics_register_types(void)
> }
> type_init(pt_graphics_register_types)
>
> +static void xen_pt_get_host_pch_info(PCIDevice *dev, uint16_t *pch_dev_id,
> + uint8_t *pch_rev_id, Error **errp)
> +{
> + FILE *fp1, *fp2;
> + char *endptr;
> + char device_id[7], rev[5];
> + size_t len;
> + const char *device = "/sys/bus/pci/devices/0000:00:1f.0/device";
> + const char *revision = "/sys/bus/pci/devices/0000:00:1f.0/revision";
> + unsigned long val;
> +
> + fp1 = fopen(device, "r");
> + if (fp1 == NULL) {
> + error_setg_errno(errp, errno, "Cannot open %s", device);
> + return;
> + }
> + fp2 = fopen(revision, "r");
> + if (fp2 == NULL) {
> + fclose(fp1);
> + error_setg_errno(errp, errno, "Cannot open %s", revision);
> + return;
> + }
> +
> + len = fread(device_id, 1, 7, fp1);
> + if (!len) {
> + fclose(fp1);
> + fclose(fp2);
> + error_setg(errp, "Cannot read %s", device);
> + return;
> + }
> + len = fread(rev, 1, 5, fp2);
> + if (!len) {
> + fclose(fp1);
> + fclose(fp2);
> + error_setg(errp, "Cannot read %s", revision);
> + return;
> + }
> + fclose(fp1);
> + fclose(fp2);
> +
> + val = strtoul(device_id, &endptr, 16);
> + if (val > 0xffff) {
> + error_setg(errp, "PCH device id is out of range: 0x%lx", val);
> + return;
> + }
> + if ((endptr > device_id) && (errno != ERANGE) &&
> + (errno != EINVAL)) {
> + *pch_dev_id = (uint16_t)val;
> + } else {
> + error_setg_errno(errp, errno, "device id strtoul "
> + "conversion failed");
> + return;
> + }
> + val = strtoul(rev, &endptr, 16);
> + if (val > 0xff) {
> + error_setg(errp, "PCH revision is out of range: 0x%lx", val);
> + return;
> + }
> + if ((endptr > rev) && (errno != ERANGE) && (errno != EINVAL)) {
> + *pch_rev_id = (uint8_t)val;
> + } else {
> + error_setg_errno(errp, errno, "revision strtoul "
> + "conversion failed");
> + }
> +}
> +
It would be better to reuse existing xen_host_pci_device_get() to read host
LPC bridge ids instead of parsing it on our own here.
> void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
> - XenHostPCIDevice *dev)
> + XenHostPCIDevice *dev,
> + Error **errp)
> {
> PCIBus *bus = pci_get_bus(&s->dev);
> struct PCIDevice *bridge_dev;
> @@ -394,7 +461,16 @@ void
> xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
> }
> }
>
> - if (pch_dev_id == 0xffff) {
> + /* Newer devices get PCH infos from host sysfs */
> + if ((pch_dev_id == 0xffff) || !pch_rev_id) {
> + xen_pt_get_host_pch_info(&s->dev, &pch_dev_id, &pch_rev_id, errp);
> + }
> +
> + XEN_PT_LOG(&s->dev, "PCH device id: 0x%x\n", pch_dev_id);
> + XEN_PT_LOG(&s->dev, "PCH revision: 0x%x\n", pch_rev_id);
> +
> + if ((pch_dev_id == 0xffff) || !pch_rev_id) {
> + error_setg(errp, "failed to get PCH device id or revision");
> return;
> }
>
> @@ -406,7 +482,7 @@ void
> xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
> * Note that vendor id is always PCI_VENDOR_ID_INTEL.
> */
> if (!bridge_dev) {
> - fprintf(stderr, "set igd-passthrough-isa-bridge failed!\n");
> + error_setg(errp, "set igd-passthrough-isa-bridge failed!");
> return;
> }
> pci_config_set_device_id(bridge_dev->config, pch_dev_id);
> diff --git a/include/hw/xen/xen_igd.h b/include/hw/xen/xen_igd.h
> index 7ffca06..da51f09 100644
> --- a/include/hw/xen/xen_igd.h
> +++ b/include/hw/xen/xen_igd.h
> @@ -22,7 +22,8 @@ uint32_t igd_read_opregion(XenPCIPassthroughState *s);
> void xen_igd_reserve_slot(PCIBus *pci_bus);
> void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
> void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
> - XenHostPCIDevice *dev);
> + XenHostPCIDevice *dev,
> + Error **errp);
>
> static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
> {
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |