> -----Original Message-----
> From: Ding, Pixel
> Sent: Monday, October 23, 2017 9:43 PM
> To: Deucher, Alexander; [email protected]
> Cc: Sun, Gary; Li, Bingley
> Subject: Re: [PATCH 4/7] drm/amdgpu/virt: add function to check MMIO
> accessing
> 
> It works with current MMIO blocking policy, while all pages are blocked
> expect the one containing mailbox registers. But actually it’s not a good
> approach, as commented we should add interaction between host and guest
> for MMIO blocking check in future.
> 
> By now I can add more comments here, is it OK?

Please add a comment that mentions what register this is.  With that fixed the 
patch is:
Acked-by: Alex Deucher <[email protected]>

> —
> Sincerely Yours,
> Pixel
> 
> 
> 
> 
> 
> 
> 
> On 24/10/2017, 12:33 AM, "Deucher, Alexander"
> <[email protected]> wrote:
> 
> >> -----Original Message-----
> >> From: amd-gfx [mailto:[email protected]] On
> Behalf
> >> Of Pixel Ding
> >> Sent: Monday, October 23, 2017 6:03 AM
> >> To: [email protected]
> >> Cc: Sun, Gary; Ding, Pixel; Li, Bingley
> >> Subject: [PATCH 4/7] drm/amdgpu/virt: add function to check MMIO
> >> accessing
> >>
> >> From: pding <[email protected]>
> >>
> >> MMIO space can be blocked on virtualised device. Add this function
> >> to check if MMIO is blocked or not.
> >>
> >> Todo: need a reliable method such like communation with hypervisor.
> >>
> >> Signed-off-by: pding <[email protected]>
> >> ---
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 5 +++++
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 1 +
> >>  2 files changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> >> index e97f80f..33dac7e 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> >> @@ -24,6 +24,11 @@
> >>  #include "amdgpu.h"
> >>  #define MAX_KIQ_REG_WAIT  100000000 /* in usecs */
> >>
> >> +bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev)
> >> +{
> >> +  return RREG32_NO_KIQ(0xc040) == 0xffffffff;
> >> +}
> >
> >Is this safe?  Won't accessing non-instanced registers cause a problem?
> Probably also worth commenting that register this is and a note for future
> asics in case the register map changes.
> >
> >Alex
> >
> >> +
> >>  int amdgpu_allocate_static_csa(struct amdgpu_device *adev)
> >>  {
> >>    int r;
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> >> index b89d37f..81efb9d 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> >> @@ -276,6 +276,7 @@ static inline bool is_virtual_machine(void)
> >>  }
> >>
> >>  struct amdgpu_vm;
> >> +bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
> >>  int amdgpu_allocate_static_csa(struct amdgpu_device *adev);
> >>  int amdgpu_map_static_csa(struct amdgpu_device *adev, struct
> >> amdgpu_vm *vm,
> >>                      struct amdgpu_bo_va **bo_va);
> >> --
> >> 2.9.5
> >>
> >> _______________________________________________
> >> amd-gfx mailing list
> >> [email protected]
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to