Re: [Qemu-devel] [v5][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support

[Chen, Tiejun]

On 2014/6/25 14:21, Paolo Bonzini wrote:
> Il 25/06/2014 04:17, Tiejun Chen ha scritto:
> > +static int get_vgabios(unsigned char *buf, XenHostPCIDevice *dev)
> > +{
> > +    char rom_file[64];
> > +    FILE *fp;
> > +    uint8_t val;
> > +    struct stat st;
> > +    uint16_t magic = 0;
> > +    int ret = 0;
> > +
> > +    snprintf(rom_file, sizeof(rom_file),
> > +             "/sys/bus/pci/devices/%04x:%02x:%02x.%d/rom",
> > +             dev->domain, dev->bus, dev->dev,
> > +             dev->func);
> > +
> > +    if (stat(rom_file, &st)) {
> > +        return -ENODEV;
> > +    }
> > +
> > +    if (access(rom_file, F_OK)) {
> > +        XEN_PT_ERR(NULL, "pci-assign: Insufficient privileges for %s",
> > +                    rom_file);
> > +        return -ENODEV;
> > +    }
>
> These two checks (stat and access) are useless.

Will remove this.

> > +    /* Write "1" to the ROM file to enable it */
> > +    fp = fopen(rom_file, "r+");
> > +    if (fp == NULL) {
> > +        return -EACCES;
> > +    }
> > +    val = 1;
> > +    if (fwrite(&val, 1, 1, fp) != 1) {
> > +        XEN_PT_LOG("%s\n", "Failed to enable pci-sysfs rom file");
> > +        ret = -EIO;
> > +        goto close_rom;
> > +    }
> > +    fseek(fp, 0, SEEK_SET);
> > +
> > +    /*
> > +     * Check if it a real bios extension.
> > +     * The magic number is 0xAA55.
> > +     */
> > +    if (!fread(&magic, sizeof(magic), 1, fp)) {
> > +        XEN_PT_ERR(NULL, "VGA: can't get magic.\n");
> > +        ret = -ENODEV;
> > +        goto close_rom;
> > +    }
> > +    if (magic != 0xAA55) {
> > +        XEN_PT_ERR(NULL, "VGA: wrong magic %x.\n", magic);
> > +        ret = -ENODEV;
> > +        goto close_rom;
> > +    }
> > +    fseek(fp, 0, SEEK_SET);
> > +
> > +    if (!fread(buf, 1, st.st_size, fp)) {
> > +        XEN_PT_ERR(NULL, "VGA: pci-assign: Cannot read from host %s",
> > rom_file);
> > +        XEN_PT_LOG(NULL, "VGA: Device option ROM contents are
> > probably invalid "
> > +                     "(check dmesg).\nSkip option ROM probe with
> > rombar=0, "
> > +                     "or load from file with romfile=\n");
> > +    }
> > +
>
> Why is this graphics passthrough specific?  Should QEMU's Xen PCI
> passthrough read ROM BAR contents from network cards too, for example?

I think this should not be same simply since we need to call VBIOS from 
RAM, not a ROM BAR of the device.

Thanks
Tiejun