Patchwork [6/8] x86_64 config: fix compile for CONFIG_VGA_ISA=n

login
register
mail settings
Submitter David S. Ahern
Date Jan. 13, 2011, 6:34 a.m.
Message ID <1294900477-23722-7-git-send-email-daahern@cisco.com>
Download mbox | patch
Permalink /patch/78666/
State New
Headers show

Comments

David S. Ahern - Jan. 13, 2011, 6:34 a.m.
Signed-off-by: David Ahern <daahern@cisco.com>
---
 hw/pc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Paolo Bonzini - Jan. 13, 2011, 5:02 p.m.
On 01/13/2011 07:34 AM, David Ahern wrote:
> Signed-off-by: David Ahern<daahern@cisco.com>
> ---
>   hw/pc.c |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index e7514fd..11b570f 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1087,8 +1087,10 @@ void pc_vga_init(PCIBus *pci_bus)
>       } else if (std_vga_enabled) {
>           if (pci_bus) {
>               pci_vga_init(pci_bus);
> +#ifdef CONFIG_VGA_ISA
>           } else {
>               isa_vga_init();
> +#endif

Should this be an abort for #ifndef CONFIG_VGA_ISA?  And maybe the isapc 
machine should be disabled altogether?

Paolo
David S. Ahern - Jan. 13, 2011, 5:28 p.m.
On 01/13/11 10:02, Paolo Bonzini wrote:
> On 01/13/2011 07:34 AM, David Ahern wrote:
>> Signed-off-by: David Ahern<daahern@cisco.com>
>> ---
>>   hw/pc.c |    2 ++
>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index e7514fd..11b570f 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -1087,8 +1087,10 @@ void pc_vga_init(PCIBus *pci_bus)
>>       } else if (std_vga_enabled) {
>>           if (pci_bus) {
>>               pci_vga_init(pci_bus);
>> +#ifdef CONFIG_VGA_ISA
>>           } else {
>>               isa_vga_init();
>> +#endif
> 
> Should this be an abort for #ifndef CONFIG_VGA_ISA?  And maybe the isapc
> machine should be disabled altogether?
> 
> Paolo

If that's desired I can change that to:

#ifdef CONFIG_VGA_ISA
         } else {
             isa_vga_init();
#else
             abort();
#endif


David
Paolo Bonzini - Jan. 14, 2011, 7:31 a.m.
On 01/13/2011 06:28 PM, David Ahern wrote:
>>> @@ -1087,8 +1087,10 @@ void pc_vga_init(PCIBus *pci_bus)
>>>        } else if (std_vga_enabled) {
>>>            if (pci_bus) {
>>>                pci_vga_init(pci_bus);
>>> +#ifdef CONFIG_VGA_ISA
>>>            } else {
>>>                isa_vga_init();
>>> +#endif
>>
>> Should this be an abort for #ifndef CONFIG_VGA_ISA?  And maybe the isapc
>> machine should be disabled altogether?
>>
>> Paolo
>
> If that's desired I can change that to:
>
> #ifdef CONFIG_VGA_ISA
>           } else {
>               isa_vga_init();
> #else
>               abort();
> #endif

Please check if you can trigger the abort via -M isapc (I think so, but 
I'm not sure).  If I am correct, the better solution is to disable isapc.

Paolo
David S. Ahern - Jan. 14, 2011, 5:49 p.m.
On 01/14/11 00:31, Paolo Bonzini wrote:
> On 01/13/2011 06:28 PM, David Ahern wrote:
>>>> @@ -1087,8 +1087,10 @@ void pc_vga_init(PCIBus *pci_bus)
>>>>        } else if (std_vga_enabled) {
>>>>            if (pci_bus) {
>>>>                pci_vga_init(pci_bus);
>>>> +#ifdef CONFIG_VGA_ISA
>>>>            } else {
>>>>                isa_vga_init();
>>>> +#endif
>>>
>>> Should this be an abort for #ifndef CONFIG_VGA_ISA?  And maybe the isapc
>>> machine should be disabled altogether?
>>>
>>> Paolo
>>
>> If that's desired I can change that to:
>>
>> #ifdef CONFIG_VGA_ISA
>>           } else {
>>               isa_vga_init();
>> #else
>>               abort();
>> #endif
> 
> Please check if you can trigger the abort via -M isapc (I think so, but
> I'm not sure).  If I am correct, the better solution is to disable isapc.
> 
> Paolo

Yes, it would trigger the abort.

And, that test case exposed a larger issue: config-devices.mak and
config-all-devices.mak are not added to the config-host.h and
x86_64-softmmu/config-target.h config files which means the addition of
the ifdef makes it disabled all the time -- regardless of the y/n
setting in the config*.mak file. ie., a patch is needed to get the
config*device.mak options added to the config*.h file.

David

Patch

diff --git a/hw/pc.c b/hw/pc.c
index e7514fd..11b570f 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1087,8 +1087,10 @@  void pc_vga_init(PCIBus *pci_bus)
     } else if (std_vga_enabled) {
         if (pci_bus) {
             pci_vga_init(pci_bus);
+#ifdef CONFIG_VGA_ISA
         } else {
             isa_vga_init();
+#endif
         }
     }
 }