Re: ATI/AMD VGA passthru report

["Pavel Mateja" <pavel@netsafe.cz>]

> On Mon, May 07, 2012 at 01:45:23PM +0200, Pavel Matěja wrote:
>> Hi,
>> I'm trying to run AMD VGA passthru on latest XEN unstable with latest
>> kernel.
>> I already have working setup with ancient xen-devel 4.2 and 2.6.32.x
>> xenified
>> kernel. See attached log.
>>
>> So I tried just to update and patch xen and kernel, but I had no luck so
>> far.
>>
>> Xen has ati_vbios_patch_respin.txt sent to xen-devel by Wei Huang
>> recently.
>> http://lists.xen.org/archives/html/xen-devel/2012-04/msg00291.html
>>
>> I tried two kernels
>> testing-3.5-with-extra and xen/next-3.2 from
>> git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git
>> and
>> git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git
>> Both with ioperm opcode patch which is required by the ATI patch. See
>> attachement.
>>
>> The xen/next-3.2 branch kernel was able to start booting windows.
>> With Catalyst driver installed I just saw bluescreen during Windows
>> boot.
>> Without Catalyst driver Windows were able to boot but Windows ended
>> frozen or
>> USB was not working. It was hard to tell because input devices had no
>> response
>> at all.
>>
>> The testing-3.5-with-extra (reported as 3.4.0-rc4) just crashed dom0
>> kernel
>> during Windows boot. I guess I have to rework the io_bitmap patch
>> somehow.
>
> When you say 'io_bitmap' patch are you referring to these ones:
>
> 70a357d xen: implement IO permission bitmap
> 0c596c5 x86/paravirt: paravirtualize IO permission bitmap
> 93b7a2a x86: demacro set_iopl_mask()
>
> ? I don't think those are in that tag. They are in the kitchensink -
> #testing
> - does it work with that branch?
>
> The next-3.2 is a bit old. I should actually delete it.

Hi,
I mean this patch below.
I think it was originally here:
http://old-list-archives.xen.org/archives/html/xen-devel/2009-05/msg01139.html
Then I was told by you to use devel/ioperm branch which I think doesn't
exist anymore:
http://old-list-archives.xen.org/archives/html/xen-devel/2011-11/msg01213.html

I'm too tired to test #testing branch now. I'll try it tomorrow after some
sleep.

>> diff --git a/arch/x86/include/asm/paravirt.h
>> b/arch/x86/include/asm/paravirt.h
>> index aa0f913..259ef7f 100644
>> --- a/arch/x86/include/asm/paravirt.h
>> +++ b/arch/x86/include/asm/paravirt.h
>> @@ -340,11 +340,18 @@ static inline void write_idt_entry(gate_desc *dt,
>> int entry, const gate_desc *g)
>>  {
>>  	PVOP_VCALL3(pv_cpu_ops.write_idt_entry, dt, entry, g);
>>  }
>> +
>>  static inline void set_iopl_mask(unsigned mask)
>>  {
>>  	PVOP_VCALL1(pv_cpu_ops.set_iopl_mask, mask);
>>  }
>>
>> +static inline void set_io_bitmap(struct thread_struct *thread,
>> +                                unsigned long bytes_updated)
>> +{
>> +       PVOP_VCALL2(pv_cpu_ops.set_io_bitmap, thread, bytes_updated);
>> +}
>> +
>>  /* The paravirtualized I/O functions */
>>  static inline void slow_down_io(void)
>>  {
>> diff --git a/arch/x86/include/asm/paravirt_types.h
>> b/arch/x86/include/asm/paravirt_types.h
>> index 8e8b9a4..96f267c 100644
>> --- a/arch/x86/include/asm/paravirt_types.h
>> +++ b/arch/x86/include/asm/paravirt_types.h
>> @@ -142,6 +142,8 @@ struct pv_cpu_ops {
>>  	void (*load_sp0)(struct tss_struct *tss, struct thread_struct *t);
>>
>>  	void (*set_iopl_mask)(unsigned mask);
>> +        void (*set_io_bitmap)(struct thread_struct *thread,
>> +                              unsigned long bytes_updated);
>>
>>  	void (*wbinvd)(void);
>>  	void (*io_delay)(void);
>> diff --git a/arch/x86/include/asm/processor.h
>> b/arch/x86/include/asm/processor.h
>> index 4fa7dcc..62becce 100644
>> --- a/arch/x86/include/asm/processor.h
>> +++ b/arch/x86/include/asm/processor.h
>> @@ -503,6 +503,9 @@ static inline void native_set_iopl_mask(unsigned
>> mask)
>>  #endif
>>  }
>>
>> +extern void native_set_io_bitmap(struct thread_struct *thread,
>> +                                unsigned long updated_bytes);
>> +
>>  static inline void
>>  native_load_sp0(struct tss_struct *tss, struct thread_struct *thread)
>>  {
>> @@ -536,6 +539,7 @@ static inline void load_sp0(struct tss_struct *tss,
>>  }
>>
>>  #define set_iopl_mask native_set_iopl_mask
>> +#define set_io_bitmap native_set_io_bitmap
>>  #endif /* CONFIG_PARAVIRT */
>>
>>  /*
>> diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
>> index 8c96897..c372ef0 100644
>> --- a/arch/x86/kernel/ioport.c
>> +++ b/arch/x86/kernel/ioport.c
>> @@ -17,13 +17,29 @@
>>  #include <linux/bitmap.h>
>>  #include <asm/syscalls.h>
>>
>> +void native_set_io_bitmap(struct thread_struct *t,
>> +                         unsigned long bytes_updated)
>> +{
>> +       struct tss_struct *tss;
>> +
>> +       if (!bytes_updated)
>> +               return;
>> +
>> +       tss = &__get_cpu_var(init_tss);
>> +
>> +       /* Update the TSS: */
>> +       if (t->io_bitmap_ptr)
>> +               memcpy(tss->io_bitmap, t->io_bitmap_ptr, bytes_updated);
>> +       else
>> +               memset(tss->io_bitmap, 0xff, bytes_updated);
>> +}
>> +
>>  /*
>>   * this changes the io permissions bitmap in the current task.
>>   */
>>  asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int
>> turn_on)
>>  {
>>  	struct thread_struct *t = &current->thread;
>> -	struct tss_struct *tss;
>>  	unsigned int i, max_long, bytes, bytes_updated;
>>
>>  	if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
>> @@ -48,13 +64,13 @@ asmlinkage long sys_ioperm(unsigned long from,
>> unsigned long num, int turn_on)
>>  	}
>>
>>  	/*
>> -	 * do it in the per-thread copy and in the TSS ...
>> +	 * do it in the per-thread copy
>>  	 *
>> -	 * Disable preemption via get_cpu() - we must not switch away
>> +	 * Disable preemption - we must not switch away
>>  	 * because the ->io_bitmap_max value must match the bitmap
>>  	 * contents:
>>  	 */
>> -	tss = &per_cpu(init_tss, get_cpu());
>> +        preempt_disable();
>>
>>  	if (turn_on)
>>  		bitmap_clear(t->io_bitmap_ptr, from, num);
>> @@ -75,10 +91,9 @@ asmlinkage long sys_ioperm(unsigned long from,
>> unsigned long num, int turn_on)
>>
>>  	t->io_bitmap_max = bytes;
>>
>> -	/* Update the TSS: */
>> -	memcpy(tss->io_bitmap, t->io_bitmap_ptr, bytes_updated);
>> +        set_io_bitmap(t, bytes_updated);
>>
>> -	put_cpu();
>> +        preempt_enable();
>>
>>  	return 0;
>>  }
>> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
>> index ab13760..aa420e7 100644
>> --- a/arch/x86/kernel/paravirt.c
>> +++ b/arch/x86/kernel/paravirt.c
>> @@ -391,6 +391,7 @@ struct pv_cpu_ops pv_cpu_ops = {
>>  	.swapgs = native_swapgs,
>>
>>  	.set_iopl_mask = native_set_iopl_mask,
>> +        .set_io_bitmap = native_set_io_bitmap,
>>  	.io_delay = native_io_delay,
>>
>>  	.start_context_switch = paravirt_nop,
>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
>> index 1d92a5a..e8436cc 100644
>> --- a/arch/x86/kernel/process.c
>> +++ b/arch/x86/kernel/process.c
>> @@ -91,16 +91,12 @@ void exit_thread(void)
>>  	unsigned long *bp = t->io_bitmap_ptr;
>>
>>  	if (bp) {
>> -		struct tss_struct *tss = &per_cpu(init_tss, get_cpu());
>> -
>> +                preempt_disable();
>>  		t->io_bitmap_ptr = NULL;
>>  		clear_thread_flag(TIF_IO_BITMAP);
>> -		/*
>> -		 * Careful, clear this in the TSS too:
>> -		 */
>> -		memset(tss->io_bitmap, 0xff, t->io_bitmap_max);
>> +                set_io_bitmap(t, t->io_bitmap_max);
>>  		t->io_bitmap_max = 0;
>> -		put_cpu();
>> +                preempt_enable();
>>  		kfree(bp);
>>  	}
>>  }
>> @@ -237,19 +233,11 @@ void __switch_to_xtra(struct task_struct *prev_p,
>> struct task_struct *next_p,
>>  			hard_enable_TSC();
>>  	}
>>
>> -	if (test_tsk_thread_flag(next_p, TIF_IO_BITMAP)) {
>> -		/*
>> -		 * Copy the relevant range of the IO bitmap.
>> -		 * Normally this is 128 bytes or less:
>> -		 */
>> -		memcpy(tss->io_bitmap, next->io_bitmap_ptr,
>> -		       max(prev->io_bitmap_max, next->io_bitmap_max));
>> -	} else if (test_tsk_thread_flag(prev_p, TIF_IO_BITMAP)) {
>> -		/*
>> -		 * Clear any possible leftover bits:
>> -		 */
>> -		memset(tss->io_bitmap, 0xff, prev->io_bitmap_max);
>> -	}
>> +        if (test_tsk_thread_flag(next_p, TIF_IO_BITMAP) ||
>> +            test_tsk_thread_flag(prev_p, TIF_IO_BITMAP))
>> +                set_io_bitmap(next,
>> +                              max(prev->io_bitmap_max,
>> next->io_bitmap_max));
>> +
>>  	propagate_user_return_notify(prev_p, next_p);
>>  }
>>
>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>> index 8853204..7a05509 100644
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>> @@ -808,6 +808,18 @@ static void xen_set_iopl_mask(unsigned mask)
>>  	HYPERVISOR_physdev_op(PHYSDEVOP_set_iopl, &set_iopl);
>>  }
>>
>> +static void xen_set_io_bitmap(struct thread_struct *thread,
>> +	int changed, unsigned long bytes_updated)
>> +{
>> +	struct physdev_set_iobitmap set_iobitmap;
>> +
>> +	set_xen_guest_handle(set_iobitmap.bitmap,
>> +		(char *)thread->io_bitmap_ptr);
>> +	set_iobitmap.nr_ports = thread->io_bitmap_ptr ? IO_BITMAP_BITS : 0;
>> +	WARN_ON(HYPERVISOR_physdev_op(PHYSDEVOP_set_iobitmap,
>> +		&set_iobitmap));
>> +}
>> +
>>  static void xen_io_delay(void)
>>  {
>>  }
>> @@ -1117,6 +1129,7 @@ static const struct pv_cpu_ops xen_cpu_ops
>> __initconst = {
>>  	.load_sp0 = xen_load_sp0,
>>
>>  	.set_iopl_mask = xen_set_iopl_mask,
>> +	.set_io_bitmap = xen_set_io_bitmap,
>>  	.io_delay = xen_io_delay,
>>
>>  	/* Xen takes care of %gs when switching to usermode for us */

-- 
Pavel Mateja