[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapp
From: |
Chen, Tiejun |
Subject: |
Re: [Qemu-devel] [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping |
Date: |
Thu, 29 May 2014 01:38:13 +0000 |
> -----Original Message-----
> From: Stefano Stabellini [mailto:address@hidden
> Sent: Wednesday, May 28, 2014 8:24 PM
> To: Chen, Tiejun
> Cc: Stefano Stabellini; address@hidden; address@hidden;
> address@hidden; address@hidden;
> address@hidden; address@hidden;
> address@hidden; Kay, Allen M; Zhang, Yang Z
> Subject: RE: [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping
>
> On Wed, 28 May 2014, Chen, Tiejun wrote:
> > > -----Original Message-----
> > > From: Stefano Stabellini [mailto:address@hidden
> > > Sent: Wednesday, May 28, 2014 1:36 AM
> > > To: Chen, Tiejun
> > > Cc: address@hidden; address@hidden;
> > > address@hidden; address@hidden; address@hidden;
> > > address@hidden; address@hidden;
> > > address@hidden; Kay, Allen M; Zhang, Yang Z
> > > Subject: Re: [v3][PATCH 5/5] xen, gfx passthrough: add opregion
> > > mapping
> > >
[snip]
> > > > +
> > > > +uint32_t igd_read_opregion(XenPCIPassthroughState *s) {
> > > > + uint32_t val = 0;
> > > > +
> > > > + if (igd_guest_opregion == 0) {
> > > > + return val;
> > >
> > > Sorry for not having spotted it before, but isn't this supposed to return
> > > an
> error?
> > > The old code returns -1 in this case.
> >
> > I already commented this in v2 log above.
> >
> > We shouldn't return "-1" to indicate an invalid address since we often think
> "!0" is a valid address value.
> >
> > In Linux instance,
> >
> > drivers/gpu/drm/i915/intel_opregion.c:
> >
> > int intel_opregion_setup(struct drm_device *dev) {
> > ...
> > pci_read_config_dword(dev->pdev, PCI_ASLS, &asls);
> > DRM_DEBUG_DRIVER("graphic opregion physical addr: 0x%x\n", asls);
> > if (asls == 0) {
> > DRM_DEBUG_DRIVER("ACPI OpRegion not supported!\n");
> > return -ENOTSUPP;
> > }
> > ...
>
> Ops, you are right! igd_read_opregion returns an address not an error code.
> In that case, given that xen_pt_intel_opregion_read can return an error code
> as well as an address, maybe it makes sense to do the same in
> igd_read_opregion and allow the function to return an error code and set the
> value by pointer.
>
I don't think so.
As I understand, we does check if that return value is valid, but not check if
this read behavior is good. This pci read transmit should be guaranteed by the
hardware, and this should be transparent to driver.
In qemu emulator, qemu should do this check on behavior of the hardware so we
need an error code when use xen_pt_intel_oprerion() as a wrapper of
igd_read_opregion(). But that return value we're talking about should be
delivered directly if this pci read behavior is fine.
But even if this read is failed, we should do something in pci host controller
of qemu to further emulate this issue. But as emulator, I think qemu often
never do this thing just specific to a normal pci read if you have no any
obvious reason.
Thanks
Tiejun
- Re: [Qemu-devel] [Xen-devel] [v3][PATCH 2/5] xen, gfx passthrough: create intel isa bridge, (continued)
- [Qemu-devel] [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Tiejun Chen, 2014/05/26
- Re: [Qemu-devel] [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Stefano Stabellini, 2014/05/27
- Re: [Qemu-devel] [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Chen, Tiejun, 2014/05/27
- Re: [Qemu-devel] [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Stefano Stabellini, 2014/05/28
- Re: [Qemu-devel] [v3][PATCH 3/5] xen, gfx passthrough: support Intel IGD passthrough with VT-D, Chen, Tiejun, 2014/05/28
[Qemu-devel] [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping, Tiejun Chen, 2014/05/26
- Re: [Qemu-devel] [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping, Stefano Stabellini, 2014/05/27
- Re: [Qemu-devel] [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping, Chen, Tiejun, 2014/05/27
- Re: [Qemu-devel] [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping, Stefano Stabellini, 2014/05/28
- Re: [Qemu-devel] [v3][PATCH 5/5] xen, gfx passthrough: add opregion mapping,
Chen, Tiejun <=
[Qemu-devel] [v3][PATCH 4/5] xen, gfx passthrough: create host bridge to passthrough, Tiejun Chen, 2014/05/26
Re: [Qemu-devel] [v3][PATCH 0/5] xen: add Intel IGD passthrough support, Chen, Tiejun, 2014/05/27