[PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Noboru Iwamatsu]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

Hi,

According to the "PCI Firmware Spec Rev 3.0",
the system firmware have to write the new checksum after resizing
the expansion ROM area.

So, when re-using the shadowed VGABIOS, the checksum must
be valid, and if it is invalid, we should consider the BIOS has
a bug or memory area is corrupted.

This patch just add the warning message when checksum requires
recalculation.

I tried this following environments.

Intel DX58SO + GeForce GTS250:	checksum is OK.
Intel DX58SO + GeForce 9600GT:	checksum is OK.
Fujitsu Q35 M/B + IGD:		checksum is bad.

What do you think, Weidong?
Is just warning enough? Or, should we stop loading the rom?

Regards,
Noboru.

[-- Attachment #2: vgabios-checksum-warn.patch --]
[-- Type: text/plain, Size: 464 bytes --]

diff --git a/hw/pass-through.c b/hw/pass-through.c
index ecb3d6f..fadd358 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -4262,7 +4262,10 @@ static int setup_vga_pt(void)
     for ( c = (char*)bios; c < ((char*)bios + bios_size); c++ )
         checksum += *c;
     if ( checksum )
+    {
         bios[bios_size - 1] -= checksum;
+        PT_LOG("vga bios checksum is adjusted!\n");
+    }
 
     cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Weidong Han]

Noboru Iwamatsu wrote:
> Hi,
>
> According to the "PCI Firmware Spec Rev 3.0",
> the system firmware have to write the new checksum after resizing
> the expansion ROM area.
>
> So, when re-using the shadowed VGABIOS, the checksum must
> be valid, and if it is invalid, we should consider the BIOS has
> a bug or memory area is corrupted.
>
> This patch just add the warning message when checksum requires
> recalculation.
>
> I tried this following environments.
>
> Intel DX58SO + GeForce GTS250:	checksum is OK.
> Intel DX58SO + GeForce 9600GT:	checksum is OK.
> Fujitsu Q35 M/B + IGD:		checksum is bad.
>   
What does "checksum is bad" mean here?
> What do you think, Weidong?
> Is just warning enough? Or, should we stop loading the rom?
>
>   
Does it still work or not when checksum is bad? If it works, we should
not stop loading the rom.

Regards,
Weidong
> Regards,
> Noboru.
>   

Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Noboru Iwamatsu]

Hi,

 >> Intel DX58SO + GeForce GTS250:	checksum is OK.
 >> Intel DX58SO + GeForce 9600GT:	checksum is OK.
 >> Fujitsu Q35 M/B + IGD:		checksum is bad.
 >>
 > What does "checksum is bad" mean here?

It means shadowed VGABIOS has invalid checksum
and requires recalculating.

 >> What do you think, Weidong?
 >> Is just warning enough? Or, should we stop loading the rom?
 >>
 >>
 > Does it still work or not when checksum is bad? If it works, we should
 > not stop loading the rom.

When the checksum is bad,
if it is not recalculated, guest has some trouble about handling the
VGA (unable to show BIOS screen, unable to load the gfx driver
properly, ...).
If it is recalculated, it seems to work fine.

Regards,
Noboru.

> Noboru Iwamatsu wrote:
>> Hi,
>>
>> According to the "PCI Firmware Spec Rev 3.0",
>> the system firmware have to write the new checksum after resizing
>> the expansion ROM area.
>>
>> So, when re-using the shadowed VGABIOS, the checksum must
>> be valid, and if it is invalid, we should consider the BIOS has
>> a bug or memory area is corrupted.
>>
>> This patch just add the warning message when checksum requires
>> recalculation.
>>
>> I tried this following environments.
>>
>> Intel DX58SO + GeForce GTS250:	checksum is OK.
>> Intel DX58SO + GeForce 9600GT:	checksum is OK.
>> Fujitsu Q35 M/B + IGD:		checksum is bad.
>>
> What does "checksum is bad" mean here?
>> What do you think, Weidong?
>> Is just warning enough? Or, should we stop loading the rom?
>>
>>
> Does it still work or not when checksum is bad? If it works, we should
> not stop loading the rom.
>
> Regards,
> Weidong
>> Regards,
>> Noboru.
>>
>

Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Weidong Han]

Noboru Iwamatsu wrote:
> Hi,
>
>  >> Intel DX58SO + GeForce GTS250:	checksum is OK.
>  >> Intel DX58SO + GeForce 9600GT:	checksum is OK.
>  >> Fujitsu Q35 M/B + IGD:		checksum is bad.
>  >>
>  > What does "checksum is bad" mean here?
>
> It means shadowed VGABIOS has invalid checksum
> and requires recalculating.
>
>  >> What do you think, Weidong?
>  >> Is just warning enough? Or, should we stop loading the rom?
>  >>
>  >>
>  > Does it still work or not when checksum is bad? If it works, we should
>  > not stop loading the rom.
>
> When the checksum is bad,
> if it is not recalculated, guest has some trouble about handling the
> VGA (unable to show BIOS screen, unable to load the gfx driver
> properly, ...).
For this case, it's reasonable to warning and stop loading rom. We 
didn't encounter this issue.
> If it is recalculated, it seems to work fine.
>   
How did you recalculate it? is it possible to recalculate it when detect 
invalid checksum?

Regards,
Weidong

> Regards,
> Noboru.
>
>   
>> Noboru Iwamatsu wrote:
>>     
>>> Hi,
>>>
>>> According to the "PCI Firmware Spec Rev 3.0",
>>> the system firmware have to write the new checksum after resizing
>>> the expansion ROM area.
>>>
>>> So, when re-using the shadowed VGABIOS, the checksum must
>>> be valid, and if it is invalid, we should consider the BIOS has
>>> a bug or memory area is corrupted.
>>>
>>> This patch just add the warning message when checksum requires
>>> recalculation.
>>>
>>> I tried this following environments.
>>>
>>> Intel DX58SO + GeForce GTS250:	checksum is OK.
>>> Intel DX58SO + GeForce 9600GT:	checksum is OK.
>>> Fujitsu Q35 M/B + IGD:		checksum is bad.
>>>
>>>       
>> What does "checksum is bad" mean here?
>>     
>>> What do you think, Weidong?
>>> Is just warning enough? Or, should we stop loading the rom?
>>>
>>>
>>>       
>> Does it still work or not when checksum is bad? If it works, we should
>> not stop loading the rom.
>>
>> Regards,
>> Weidong
>>     
>>> Regards,
>>> Noboru.
>>>
>>>       
>
>
>   

Re: Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Noboru Iwamatsu]

[-- Attachment #1: Type: text/plain, Size: 2942 bytes --]

Hi Weidong,

 > How did you recalculate it? is it possible to recalculate it when detect
 > invalid checksum?

Checking and recalculating the VGABIOS checksum is being processed
in setup_vga_pt() in hw/pass-through.c.
I thought you have written that part.

If the checksum is invalid, current setup_vga_pt() always overwrites
it with the recalculated value.

What I mention is: the VGABIOS checksum must have been already
recalculated by system BIOS. So, when setup_vga_pt() detects the
invalid one, something might be wrong.

But, I'm not sure all BIOS should work as described in PCIFW Spec.
Actually, my Q35 had invalid checksum.


I attach another patch:
This stops loading vgabios if the checksum is wrong.

Signed-off-by: Noboru Iwamatsu <n_iwamatsu@jp.fujitsu.com>

> Noboru Iwamatsu wrote:
>> Hi,
>>
>> >> Intel DX58SO + GeForce GTS250: checksum is OK.
>> >> Intel DX58SO + GeForce 9600GT: checksum is OK.
>> >> Fujitsu Q35 M/B + IGD: checksum is bad.
>> >>
>> > What does "checksum is bad" mean here?
>>
>> It means shadowed VGABIOS has invalid checksum
>> and requires recalculating.
>>
>> >> What do you think, Weidong?
>> >> Is just warning enough? Or, should we stop loading the rom?
>> >>
>> >>
>> > Does it still work or not when checksum is bad? If it works, we should
>> > not stop loading the rom.
>>
>> When the checksum is bad,
>> if it is not recalculated, guest has some trouble about handling the
>> VGA (unable to show BIOS screen, unable to load the gfx driver
>> properly, ...).
> For this case, it's reasonable to warning and stop loading rom. We
> didn't encounter this issue.
>> If it is recalculated, it seems to work fine.
> How did you recalculate it? is it possible to recalculate it when detect
> invalid checksum?
>
> Regards,
> Weidong
>
>> Regards,
>> Noboru.
>>
>>> Noboru Iwamatsu wrote:
>>>> Hi,
>>>>
>>>> According to the "PCI Firmware Spec Rev 3.0",
>>>> the system firmware have to write the new checksum after resizing
>>>> the expansion ROM area.
>>>>
>>>> So, when re-using the shadowed VGABIOS, the checksum must
>>>> be valid, and if it is invalid, we should consider the BIOS has
>>>> a bug or memory area is corrupted.
>>>>
>>>> This patch just add the warning message when checksum requires
>>>> recalculation.
>>>>
>>>> I tried this following environments.
>>>>
>>>> Intel DX58SO + GeForce GTS250: checksum is OK.
>>>> Intel DX58SO + GeForce 9600GT: checksum is OK.
>>>> Fujitsu Q35 M/B + IGD: checksum is bad.
>>>>
>>> What does "checksum is bad" mean here?
>>>> What do you think, Weidong?
>>>> Is just warning enough? Or, should we stop loading the rom?
>>>>
>>>>
>>> Does it still work or not when checksum is bad? If it works, we should
>>> not stop loading the rom.
>>>
>>> Regards,
>>> Weidong
>>>> Regards,
>>>> Noboru.
>>>>
>>
>>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


[-- Attachment #2: vgabios-checksum-validation.patch --]
[-- Type: text/plain, Size: 603 bytes --]

diff --git a/hw/pass-through.c b/hw/pass-through.c
index ecb3d6f..8775956 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -4258,11 +4258,15 @@ static int setup_vga_pt(void)
         goto out;
     }
 
-    /* Adjust the bios checksum */
+    /* Validate the bios checksum */
     for ( c = (char*)bios; c < ((char*)bios + bios_size); c++ )
         checksum += *c;
     if ( checksum )
-        bios[bios_size - 1] -= checksum;
+    {
+        PT_LOG("vga bios checksum is invalid!\n");
+        rc = -1;
+        goto out;
+    }
 
     cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Weidong Han]

Noboru Iwamatsu wrote:
> Hi Weidong,
>
>  > How did you recalculate it? is it possible to recalculate it when detect
>  > invalid checksum?
>
> Checking and recalculating the VGABIOS checksum is being processed
> in setup_vga_pt() in hw/pass-through.c.
> I thought you have written that part.
>
> If the checksum is invalid, current setup_vga_pt() always overwrites
> it with the recalculated value.
>
> What I mention is: the VGABIOS checksum must have been already
> recalculated by system BIOS. So, when setup_vga_pt() detects the
> invalid one, something might be wrong.
>
> But, I'm not sure all BIOS should work as described in PCIFW Spec.
> Actually, my Q35 had invalid checksum.
>   
Now I understand. Because your Q35 works with recalculated checksum, I 
prefer to only add a warning message, and continue to load rom for gfx 
passthru.

Regards,
Weidong
>
> I attach another patch:
> This stops loading vgabios if the checksum is wrong.
>
> Signed-off-by: Noboru Iwamatsu <n_iwamatsu@jp.fujitsu.com>
>
>   
>> Noboru Iwamatsu wrote:
>>     
>>> Hi,
>>>
>>>       
>>>>> Intel DX58SO + GeForce GTS250: checksum is OK.
>>>>> Intel DX58SO + GeForce 9600GT: checksum is OK.
>>>>> Fujitsu Q35 M/B + IGD: checksum is bad.
>>>>>
>>>>>           
>>>> What does "checksum is bad" mean here?
>>>>         
>>> It means shadowed VGABIOS has invalid checksum
>>> and requires recalculating.
>>>
>>>       
>>>>> What do you think, Weidong?
>>>>> Is just warning enough? Or, should we stop loading the rom?
>>>>>
>>>>>
>>>>>           
>>>> Does it still work or not when checksum is bad? If it works, we should
>>>> not stop loading the rom.
>>>>         
>>> When the checksum is bad,
>>> if it is not recalculated, guest has some trouble about handling the
>>> VGA (unable to show BIOS screen, unable to load the gfx driver
>>> properly, ...).
>>>       
>> For this case, it's reasonable to warning and stop loading rom. We
>> didn't encounter this issue.
>>     
>>> If it is recalculated, it seems to work fine.
>>>       
>> How did you recalculate it? is it possible to recalculate it when detect
>> invalid checksum?
>>
>> Regards,
>> Weidong
>>
>>     
>>> Regards,
>>> Noboru.
>>>
>>>       
>>>> Noboru Iwamatsu wrote:
>>>>         
>>>>> Hi,
>>>>>
>>>>> According to the "PCI Firmware Spec Rev 3.0",
>>>>> the system firmware have to write the new checksum after resizing
>>>>> the expansion ROM area.
>>>>>
>>>>> So, when re-using the shadowed VGABIOS, the checksum must
>>>>> be valid, and if it is invalid, we should consider the BIOS has
>>>>> a bug or memory area is corrupted.
>>>>>
>>>>> This patch just add the warning message when checksum requires
>>>>> recalculation.
>>>>>
>>>>> I tried this following environments.
>>>>>
>>>>> Intel DX58SO + GeForce GTS250: checksum is OK.
>>>>> Intel DX58SO + GeForce 9600GT: checksum is OK.
>>>>> Fujitsu Q35 M/B + IGD: checksum is bad.
>>>>>
>>>>>           
>>>> What does "checksum is bad" mean here?
>>>>         
>>>>> What do you think, Weidong?
>>>>> Is just warning enough? Or, should we stop loading the rom?
>>>>>
>>>>>
>>>>>           
>>>> Does it still work or not when checksum is bad? If it works, we should
>>>> not stop loading the rom.
>>>>
>>>> Regards,
>>>> Weidong
>>>>         
>>>>> Regards,
>>>>> Noboru.
>>>>>
>>>>>           
>>>       
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>>     
>
>   

Re: Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Ian Jackson]

Weidong Han writes ("Re: [Xen-devel] Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum"):
> Now I understand. Because your Q35 works with recalculated checksum, I 
> prefer to only add a warning message, and continue to load rom for gfx 
> passthru.

Having read this thread I'm still a bit confused.  The problem is that
the VGA BIOS on the graphics card is broken and has a broken checksum,
and the proposed workaround is to recalculate the checksum for the
benefit of the guest ?

Does this incorrectly checksummed BIOS work natively (ie without
passthrough) and if so why is passthrough different ?  Alternatively
if it doesn't work native why are we trying to make it work with
passthrough ?

On another level, Weidong, are you suggesting you'd like to see Noboru
produce a different patch which just produces a warning ?

Thanks,
Ian.

Re: Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Noboru Iwamatsu]

[-- Attachment #1: Type: text/plain, Size: 2033 bytes --]

Hi,

> Weidong Han writes ("Re: [Xen-devel] Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum"):
>> Now I understand. Because your Q35 works with recalculated checksum, I
>> prefer to only add a warning message, and continue to load rom for gfx
>> passthru.
>
> Having read this thread I'm still a bit confused.  The problem is that
> the VGA BIOS on the graphics card is broken and has a broken checksum,
> and the proposed workaround is to recalculate the checksum for the
> benefit of the guest ?

In the native environment, the VGABIOS, the expansion ROM on the
graphics card, is placed into the 0C0000h address space, and then
executed. Of course, the checksum of the ROM must be valid.

After this initialization, the system BIOS, the actual BIOS of the M/B,
can resize the expansion ROM code to reduce the amount of occupied 
space. If the system BIOS resizes it, a new checksum must be calculated
and stored in the ROM image that is on the RAM.

So, normally, shadowed VGABIOS, that is placed in 0C0000h, is already
modified and its checksum must be recalculated.

Qemu-dm copies 0C0000h's contents of the dom0 to guest's 0C0000h.
Guest re-uses dom0's used-up VGABIOS.

The problem that I mentioned is about this recalculated checksum.

System BIOS must guarantee the checksum after the resizing, but,
some M/B does not.
However, after adjusting the checksum, guest seems to work, and
current qemu-dm does so. The buggy system BIOS might just forgets
to recalculate.

Should we check strictly here?

> Does this incorrectly checksummed BIOS work natively (ie without
> passthrough) and if so why is passthrough different ?  Alternatively
> if it doesn't work native why are we trying to make it work with
> passthrough ?
>
> On another level, Weidong, are you suggesting you'd like to see Noboru
> produce a different patch which just produces a warning ?

I sent "just warning" patch on the first of this thread.
I resend it.

Noboru.

Signed-off-by: Noboru Iwamatsu <n_iwamatsu@jp.fujitsu.com>



[-- Attachment #2: vgabios-checksum-warn.patch --]
[-- Type: text/plain, Size: 464 bytes --]

diff --git a/hw/pass-through.c b/hw/pass-through.c
index ecb3d6f..fadd358 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -4262,7 +4262,10 @@ static int setup_vga_pt(void)
     for ( c = (char*)bios; c < ((char*)bios + bios_size); c++ )
         checksum += *c;
     if ( checksum )
+    {
         bios[bios_size - 1] -= checksum;
+        PT_LOG("vga bios checksum is adjusted!\n");
+    }
 
     cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Weidong Han]

Noboru Iwamatsu wrote:
> Hi,
>
>   
>> Weidong Han writes ("Re: [Xen-devel] Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum"):
>>     
>>> Now I understand. Because your Q35 works with recalculated checksum, I
>>> prefer to only add a warning message, and continue to load rom for gfx
>>> passthru.
>>>       
>> Having read this thread I'm still a bit confused.  The problem is that
>> the VGA BIOS on the graphics card is broken and has a broken checksum,
>> and the proposed workaround is to recalculate the checksum for the
>> benefit of the guest ?
>>     
>
> In the native environment, the VGABIOS, the expansion ROM on the
> graphics card, is placed into the 0C0000h address space, and then
> executed. Of course, the checksum of the ROM must be valid.
>
> After this initialization, the system BIOS, the actual BIOS of the M/B,
> can resize the expansion ROM code to reduce the amount of occupied 
> space. If the system BIOS resizes it, a new checksum must be calculated
> and stored in the ROM image that is on the RAM.
>
> So, normally, shadowed VGABIOS, that is placed in 0C0000h, is already
> modified and its checksum must be recalculated.
>
> Qemu-dm copies 0C0000h's contents of the dom0 to guest's 0C0000h.
> Guest re-uses dom0's used-up VGABIOS.
>
> The problem that I mentioned is about this recalculated checksum.
>
> System BIOS must guarantee the checksum after the resizing, but,
> some M/B does not.
> However, after adjusting the checksum, guest seems to work, and
> current qemu-dm does so. The buggy system BIOS might just forgets
> to recalculate.
>
> Should we check strictly here?
>
>   
>> Does this incorrectly checksummed BIOS work natively (ie without
>> passthrough) and if so why is passthrough different ?  Alternatively
>> if it doesn't work native why are we trying to make it work with
>> passthrough ?
>>
>> On another level, Weidong, are you suggesting you'd like to see Noboru
>> produce a different patch which just produces a warning ?
>>     
>
> I sent "just warning" patch on the first of this thread.
> I resend it.
>   
Yes, I ack this one. Acked-by: Weidong Han <weidong.han@intel.com>

Regards,
Weidong
> Noboru.
>
> Signed-off-by: Noboru Iwamatsu <n_iwamatsu@jp.fujitsu.com>
>
>
>   

Re: Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Noboru Iwamatsu]

[-- Attachment #1: Type: text/plain, Size: 3062 bytes --]

Hi,

Generally, we should validate the checksum right.

Weidong, have you seen the checksum adjusting?

I only saw it in Q35+IGD.
You know my Q35 is buggy. If this is the only issue of my Q35,
2nd patch (validate and disable) is the best way, I think.

Or, is this a IGD specific issue?
If so, checksum adjusting should be a workaround for IGD pass-through.
I assume IGD's VGABIOS is included in the system BIOS and might be
expanded to the RAM in a different way from the PCI gfx card.

I attach 3rd patch:
When invalid checksum is detected, adjust it if igd_passthru is enabled. 
In other case, stop loading.

I tried this on my Q35+IGD, the checksum has been adjusted and worked.

Do you have any idea?

Regards,
Noboru.

Signed-off-by: Noboru Iwamatsu <n_iwamatsu@jp.fujitsu.com>

> Noboru Iwamatsu wrote:
>> Hi,
>>
>>> Weidong Han writes ("Re: [Xen-devel] Re: [PATCH][RFC] gfx_passthru:
>>> warning when vgabios rom has invalid checksum"):
>>>> Now I understand. Because your Q35 works with recalculated checksum, I
>>>> prefer to only add a warning message, and continue to load rom for gfx
>>>> passthru.
>>> Having read this thread I'm still a bit confused. The problem is that
>>> the VGA BIOS on the graphics card is broken and has a broken checksum,
>>> and the proposed workaround is to recalculate the checksum for the
>>> benefit of the guest ?
>>
>> In the native environment, the VGABIOS, the expansion ROM on the
>> graphics card, is placed into the 0C0000h address space, and then
>> executed. Of course, the checksum of the ROM must be valid.
>>
>> After this initialization, the system BIOS, the actual BIOS of the M/B,
>> can resize the expansion ROM code to reduce the amount of occupied
>> space. If the system BIOS resizes it, a new checksum must be calculated
>> and stored in the ROM image that is on the RAM.
>>
>> So, normally, shadowed VGABIOS, that is placed in 0C0000h, is already
>> modified and its checksum must be recalculated.
>>
>> Qemu-dm copies 0C0000h's contents of the dom0 to guest's 0C0000h.
>> Guest re-uses dom0's used-up VGABIOS.
>>
>> The problem that I mentioned is about this recalculated checksum.
>>
>> System BIOS must guarantee the checksum after the resizing, but,
>> some M/B does not.
>> However, after adjusting the checksum, guest seems to work, and
>> current qemu-dm does so. The buggy system BIOS might just forgets
>> to recalculate.
>>
>> Should we check strictly here?
>>
>>> Does this incorrectly checksummed BIOS work natively (ie without
>>> passthrough) and if so why is passthrough different ? Alternatively
>>> if it doesn't work native why are we trying to make it work with
>>> passthrough ?
>>>
>>> On another level, Weidong, are you suggesting you'd like to see Noboru
>>> produce a different patch which just produces a warning ?
>>
>> I sent "just warning" patch on the first of this thread.
>> I resend it.
> Yes, I ack this one. Acked-by: Weidong Han <weidong.han@intel.com>
>
> Regards,
> Weidong
>> Noboru.
>>
>> Signed-off-by: Noboru Iwamatsu <n_iwamatsu@jp.fujitsu.com>
>>
>>
>



[-- Attachment #2: adjust-checksum-for-igd.patch --]
[-- Type: text/plain, Size: 806 bytes --]

diff --git a/hw/pass-through.c b/hw/pass-through.c
index ecb3d6f..3381782 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -4258,11 +4258,23 @@ static int setup_vga_pt(void)
         goto out;
     }
 
-    /* Adjust the bios checksum */
+    /* Validate the bios checksum */
     for ( c = (char*)bios; c < ((char*)bios + bios_size); c++ )
         checksum += *c;
     if ( checksum )
-        bios[bios_size - 1] -= checksum;
+    {
+        if ( igd_passthru )
+        {
+            bios[bios_size - 1] -= checksum;
+            PT_LOG("vga bios checksum is adjusted!\n");
+        }
+        else
+        {
+            PT_LOG("vga bios checksum is invalid!\n");
+            rc = -1;
+            goto out;
+        }
+    }
 
     cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
 


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: Re: [PATCH][RFC] gfx_passthru: warning when vgabios rom has invalid checksum

[Weidong Han]

Noboru Iwamatsu wrote:
> Hi,
>
> Generally, we should validate the checksum right.
>
> Weidong, have you seen the checksum adjusting?
>
> I only saw it in Q35+IGD.
> You know my Q35 is buggy. If this is the only issue of my Q35,
> 2nd patch (validate and disable) is the best way, I think.
>
> Or, is this a IGD specific issue?
> If so, checksum adjusting should be a workaround for IGD pass-through.
> I assume IGD's VGABIOS is included in the system BIOS and might be
> expanded to the RAM in a different way from the PCI gfx card.
>   
I also found the checksum is invalid on my Q45 platform, of course it 
works fine with recalculated value.
> I attach 3rd patch:
> When invalid checksum is detected, adjust it if igd_passthru is enabled. 
> In other case, stop loading.
>
> I tried this on my Q35+IGD, the checksum has been adjusted and worked.
>
> Do you have any idea?
>   
I still prefer to only add a warning message about it, and replace it 
with recalculated checksum for both IGD and discrete gfx. There might be 
some discrete gfx cards which also have invalid checksum, and it's 
likely that it still works with recalculated checksum in guest.

Regards,
Weidong
> Regards,
> Noboru.
>
> Signed-off-by: Noboru Iwamatsu <n_iwamatsu@jp.fujitsu.com>
>
>   
>> Noboru Iwamatsu wrote:
>>     
>>> Hi,
>>>
>>>       
>>>> Weidong Han writes ("Re: [Xen-devel] Re: [PATCH][RFC] gfx_passthru:
>>>> warning when vgabios rom has invalid checksum"):
>>>>         
>>>>> Now I understand. Because your Q35 works with recalculated checksum, I
>>>>> prefer to only add a warning message, and continue to load rom for gfx
>>>>> passthru.
>>>>>           
>>>> Having read this thread I'm still a bit confused. The problem is that
>>>> the VGA BIOS on the graphics card is broken and has a broken checksum,
>>>> and the proposed workaround is to recalculate the checksum for the
>>>> benefit of the guest ?
>>>>         
>>> In the native environment, the VGABIOS, the expansion ROM on the
>>> graphics card, is placed into the 0C0000h address space, and then
>>> executed. Of course, the checksum of the ROM must be valid.
>>>
>>> After this initialization, the system BIOS, the actual BIOS of the M/B,
>>> can resize the expansion ROM code to reduce the amount of occupied
>>> space. If the system BIOS resizes it, a new checksum must be calculated
>>> and stored in the ROM image that is on the RAM.
>>>
>>> So, normally, shadowed VGABIOS, that is placed in 0C0000h, is already
>>> modified and its checksum must be recalculated.
>>>
>>> Qemu-dm copies 0C0000h's contents of the dom0 to guest's 0C0000h.
>>> Guest re-uses dom0's used-up VGABIOS.
>>>
>>> The problem that I mentioned is about this recalculated checksum.
>>>
>>> System BIOS must guarantee the checksum after the resizing, but,
>>> some M/B does not.
>>> However, after adjusting the checksum, guest seems to work, and
>>> current qemu-dm does so. The buggy system BIOS might just forgets
>>> to recalculate.
>>>
>>> Should we check strictly here?
>>>
>>>       
>>>> Does this incorrectly checksummed BIOS work natively (ie without
>>>> passthrough) and if so why is passthrough different ? Alternatively
>>>> if it doesn't work native why are we trying to make it work with
>>>> passthrough ?
>>>>
>>>> On another level, Weidong, are you suggesting you'd like to see Noboru
>>>> produce a different patch which just produces a warning ?
>>>>         
>>> I sent "just warning" patch on the first of this thread.
>>> I resend it.
>>>       
>> Yes, I ack this one. Acked-by: Weidong Han <weidong.han@intel.com>
>>
>> Regards,
>> Weidong
>>     
>>> Noboru.
>>>
>>> Signed-off-by: Noboru Iwamatsu <n_iwamatsu@jp.fujitsu.com>
>>>
>>>
>>>       
>
>
>